Re: [Qemu-devel] [PATCH v3 1/3] qmp: Add command 'blockdev-backup'
Fam Zheng writes:
> On Wed, 12/17 10:36, Markus Armbruster wrote:
>> Fam Zheng writes:
>>
>> > Similar to drive-backup, but this command uses a device id as target
>> > instead of creating/opening an image file.
>> >
>> > Also add blocker on target bs, since the target is also a named device
>> > now.
>> >
>> > Add check and report error for bs == target which became possible but is
>> > an illegal case with introduction of blockdev-backup.
[...]
>> > diff --git a/blockdev.c b/blockdev.c
>> > index 57910b8..2e5068c 100644
>> > --- a/blockdev.c
>> > +++ b/blockdev.c
>> > @@ -2156,6 +2156,8 @@ void qmp_drive_backup(const char *device, const char
>> > *target,
>> > aio_context = bdrv_get_aio_context(bs);
>> > aio_context_acquire(aio_context);
>> >
>> > +/* Although backup_run has this check too, we need to use bs->drv
>> > below, so
>> > + * do an early check redundantly. */
>> > if (!bdrv_is_inserted(bs)) {
>> > error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
>> > goto out;
>> > @@ -2172,6 +2174,7 @@ void qmp_drive_backup(const char *device, const char
>> > *target,
>> > }
>> > }
>> >
>> > +/* Early check to avoid creating target */
>> > if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
>> > goto out;
>> > }
>> > @@ -2239,6 +2242,50 @@ BlockDeviceInfoList
>> > *qmp_query_named_block_nodes(Error **errp)
>> > return bdrv_named_nodes_list();
>> > }
>> >
>> > +void qmp_blockdev_backup(const char *device, const char *target,
>> > + enum MirrorSyncMode sync,
>> > + bool has_speed, int64_t speed,
>> > + bool has_on_source_error,
>> > + BlockdevOnError on_source_error,
>> > + bool has_on_target_error,
>> > + BlockdevOnError on_target_error,
>> > + Error **errp)
>> > +{
>> > +BlockDriverState *bs;
>> > +BlockDriverState *target_bs;
>> > +Error *local_err = NULL;
>> > +
>> > +if (!has_speed) {
>> > +speed = 0;
>> > +}
>> > +if (!has_on_source_error) {
>> > +on_source_error = BLOCKDEV_ON_ERROR_REPORT;
>> > +}
>> > +if (!has_on_target_error) {
>> > +on_target_error = BLOCKDEV_ON_ERROR_REPORT;
>> > +}
>> > +
>> > +bs = bdrv_find(device);
>> > +if (!bs) {
>> > +error_set(errp, QERR_DEVICE_NOT_FOUND, device);
>> > +return;
>> > +}
>> > +
>> > +target_bs = bdrv_find(target);
>> > +if (!target_bs) {
>> > +error_set(errp, QERR_DEVICE_NOT_FOUND, target);
>> > +return;
>> > +}
New use of a QERR_ macro. See below.
>> > +
>> > +bdrv_ref(target_bs);
>> > +backup_start(bs, target_bs, speed, sync, on_source_error,
>> > on_target_error,
>> > + block_job_cb, bs, &local_err);
>> > +if (local_err != NULL) {
>> > +bdrv_unref(target_bs);
>> > +error_propagate(errp, local_err);
>> > +}
>> > +}
>> > +
>> > #define DEFAULT_MIRROR_BUF_SIZE (10 << 20)
>> >
>> > void qmp_drive_mirror(const char *device, const char *target,
>> > diff --git a/qapi/block-core.json b/qapi/block-core.json
>> > index 77a0cfb..bc02bd7 100644
>> > --- a/qapi/block-core.json
>> > +++ b/qapi/block-core.json
>> > @@ -676,6 +676,41 @@
>> > '*on-target-error': 'BlockdevOnError' } }
>> >
>> > ##
>> > +# @BlockdevBackup
>> > +#
>> > +# @device: the name of the device which should be copied.
>> > +#
>> > +# @target: the name of the backup target device.
>> > +#
>> > +# @sync: what parts of the disk image should be copied to the destination
>> > +#(all the disk, only the sectors allocated in the topmost image,
>> > or
>> > +#only new I/O).
>> > +#
>> > +# @speed: #optional the maximum speed, in bytes per second. The default
>> > is 0,
>> > +# for unlimited.
>> > +#
>> > +# @on-source-error: #optional the action to take on an error on the
>> > source,
>> > +# default 'report'. 'stop' and 'enospc' can only be
>> > used
>> > +# if the block device supports io-status (see
>> > BlockInfo).
>> > +#
>> > +# @on-target-error: #optional the action to take on an error on the
>> > target,
>> > +# default 'report' (no limitations, since this applies
>> > to
>> > +# a different block device than @device).
>> > +#
>> > +# Note that @on-source-error and @on-target-error only affect background
>> > I/O.
>> > +# If an error occurs during a guest write request, the device's
>> > rerror/werror
>> > +# actions will be used.
>> > +#
>> > +# Since: 2.3
>> > +##
>> > +{ 'type': 'BlockdevBackup',
>> > + 'data': { 'device': 'str', 'target': 'str',
>> > +'sync': 'MirrorSyncMode',
>> > +'*speed': 'int',
>> > +'*on-source-error': 'BlockdevOnError',
>> > +'*on-target-error': 'BlockdevOnEr
Re: [Qemu-devel] [PATCH v4 1/3] qmp: Add command 'blockdev-backup'
Max Reitz writes:
> On 2014-12-04 at 03:29, Fam Zheng wrote:
>> Similar to drive-backup, but this command uses a device id as target
>> instead of creating/opening an image file.
>>
>> Also add blocker on target bs, since the target is also a named device
>> now.
>>
>> Add check and report error for bs == target which became possible but is
>> an illegal case with introduction of blockdev-backup.
[...]
>> diff --git a/blockdev.c b/blockdev.c
>> index 5651a8e..f1a 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
[...]
>> +if (!bs) {
>> +error_set(errp, QERR_DEVICE_NOT_FOUND, device);
>> +return;
>> +}
>> +
>> +target_bs = bdrv_find(target);
>> +if (!target_bs) {
>> +error_set(errp, QERR_DEVICE_NOT_FOUND, target);
>> +return;
>> +}
>> +
>> +bdrv_ref(target_bs);
>> +bdrv_set_aio_context(target_bs, bdrv_get_aio_context(bs));
>
> In the cover letter you said you were acquiring the AIO context but
> you're not. Something like the aio_context_acquire() call in
> qmp_drive_backup() seems missing.
The fact that I missed this in my review demonstrates that I have to pay
much more attention to AIO contexts. Thanks, Max!
Re: [Qemu-devel] [PATCH v6 0/4] qmp: Add "blockdev-backup"
Fam Zheng writes: > v6: Add Eric's rev-by in 1/4. > Address minor comments in 2/4, 3/4. > Add John's rev-by in 3/4. > > v5: Address Max's and Markus' comments: > Split patch 1. (Markus) > Fix typos and pastos. (Markus, Max) > Actually acquire aio context. (Max) > Drop unnecessary initialization of fields in blockdev_backup_prepare. > (Max) > Add "sync" in the document example. > Add Max's rev-by in patch 4. > > The existing drive-backup command accepts a target file path, but that > interface provides little flexibility on the properties of target block > device, > compared to what is possible with "blockdev-add", "drive_add" or "-drive". > > This is also a building block to allow image fleecing (creating a point in > time > snapshot and export with nbd-server-add). > > (For symmetry, blockdev-mirror will be added in a separate series.) As I pointed out a review thread of v3, PATCH 2 adds new, unwanted uses of QERR_ macros and ERROR_CLASS_DEVICE_NOT_FOUND. Can be fixed on top. Series Reviewed-by: Markus Armbruster
[Qemu-devel] [PULL 02/10] vnc: remove unused DisplayState parameter, add id instead.
DisplayState isn't used anywhere, drop it. Add the vnc server ID as
parameter instead, so it is possible to specify the server instance.
Signed-off-by: Gerd Hoffmann
Reviewed-by: Gonglei
---
include/ui/console.h | 16
ui/vnc.c | 29 ++---
vl.c | 7 ---
3 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/include/ui/console.h b/include/ui/console.h
index 22ef8ca..5ff2e27 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -327,19 +327,19 @@ void sdl_display_init(DisplayState *ds, int full_screen,
int no_frame);
void cocoa_display_init(DisplayState *ds, int full_screen);
/* vnc.c */
-void vnc_display_init(DisplayState *ds);
-void vnc_display_open(DisplayState *ds, const char *display, Error **errp);
-void vnc_display_add_client(DisplayState *ds, int csock, bool skipauth);
-char *vnc_display_local_addr(DisplayState *ds);
+void vnc_display_init(const char *id);
+void vnc_display_open(const char *id, const char *display, Error **errp);
+void vnc_display_add_client(const char *id, int csock, bool skipauth);
+char *vnc_display_local_addr(const char *id);
#ifdef CONFIG_VNC
-int vnc_display_password(DisplayState *ds, const char *password);
-int vnc_display_pw_expire(DisplayState *ds, time_t expires);
+int vnc_display_password(const char *id, const char *password);
+int vnc_display_pw_expire(const char *id, time_t expires);
#else
-static inline int vnc_display_password(DisplayState *ds, const char *password)
+static inline int vnc_display_password(const char *id, const char *password)
{
return -ENODEV;
}
-static inline int vnc_display_pw_expire(DisplayState *ds, time_t expires)
+static inline int vnc_display_pw_expire(const char *id, time_t expires)
{
return -ENODEV;
};
diff --git a/ui/vnc.c b/ui/vnc.c
index a6549c8..fce4861 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2967,10 +2967,11 @@ static const DisplayChangeListenerOps dcl_ops = {
.dpy_cursor_define = vnc_dpy_cursor_define,
};
-void vnc_display_init(DisplayState *ds)
+void vnc_display_init(const char *id)
{
VncDisplay *vs = g_malloc0(sizeof(*vs));
+vs->id = strdup(id);
QTAILQ_INSERT_TAIL(&vnc_displays, vs, next);
vs->lsock = -1;
@@ -2999,10 +3000,8 @@ void vnc_display_init(DisplayState *ds)
}
-static void vnc_display_close(DisplayState *ds)
+static void vnc_display_close(VncDisplay *vs)
{
-VncDisplay *vs = vnc_display_find(NULL);
-
if (!vs)
return;
g_free(vs->display);
@@ -3028,9 +3027,9 @@ static void vnc_display_close(DisplayState *ds)
#endif
}
-int vnc_display_password(DisplayState *ds, const char *password)
+int vnc_display_password(const char *id, const char *password)
{
-VncDisplay *vs = vnc_display_find(NULL);
+VncDisplay *vs = vnc_display_find(id);
if (!vs) {
return -EINVAL;
@@ -3047,9 +3046,9 @@ int vnc_display_password(DisplayState *ds, const char
*password)
return 0;
}
-int vnc_display_pw_expire(DisplayState *ds, time_t expires)
+int vnc_display_pw_expire(const char *id, time_t expires)
{
-VncDisplay *vs = vnc_display_find(NULL);
+VncDisplay *vs = vnc_display_find(id);
if (!vs) {
return -EINVAL;
@@ -3059,16 +3058,16 @@ int vnc_display_pw_expire(DisplayState *ds, time_t
expires)
return 0;
}
-char *vnc_display_local_addr(DisplayState *ds)
+char *vnc_display_local_addr(const char *id)
{
-VncDisplay *vs = vnc_display_find(NULL);
+VncDisplay *vs = vnc_display_find(id);
return vnc_socket_local_addr("%s:%s", vs->lsock);
}
-void vnc_display_open(DisplayState *ds, const char *display, Error **errp)
+void vnc_display_open(const char *id, const char *display, Error **errp)
{
-VncDisplay *vs = vnc_display_find(NULL);
+VncDisplay *vs = vnc_display_find(id);
const char *options;
int password = 0;
int reverse = 0;
@@ -3088,7 +3087,7 @@ void vnc_display_open(DisplayState *ds, const char
*display, Error **errp)
error_setg(errp, "VNC display not active");
return;
}
-vnc_display_close(ds);
+vnc_display_close(vs);
if (strcmp(display, "none") == 0)
return;
@@ -3381,9 +3380,9 @@ fail:
#endif /* CONFIG_VNC_WS */
}
-void vnc_display_add_client(DisplayState *ds, int csock, bool skipauth)
+void vnc_display_add_client(const char *id, int csock, bool skipauth)
{
-VncDisplay *vs = vnc_display_find(NULL);
+VncDisplay *vs = vnc_display_find(id);
if (!vs) {
return;
diff --git a/vl.c b/vl.c
index 113e98e..afb6212 100644
--- a/vl.c
+++ b/vl.c
@@ -4376,8 +4376,9 @@ int main(int argc, char **argv, char **envp)
/* init remote displays */
if (vnc_display) {
Error *local_err = NULL;
-vnc_display_init(ds);
-vnc_display_open(ds, vnc_display, &local_err);
+const char *id = "default";
+vnc_display_init(id);
+vnc_display_open(id, vnc_display, &local_err);
if
[Qemu-devel] [PULL 09/10] monitor: add query-vnc2 command
Add new query vnc qmp command, for the lack of better ideas just name it
"query-vnc2". Changes over query-vnc:
* It returns a list of vnc servers, so multiple vnc server instances
are covered.
* Each vnc server returns a list of server sockets. Followup patch
will use that to also report websockets. In case we add support for
multiple server sockets server sockets (to better support ipv4+ipv6
dualstack) we can add them to the list too.
Signed-off-by: Gerd Hoffmann
---
qapi-schema.json | 68
qmp-commands.hx | 5 +++
ui/vnc.c | 133 +++
3 files changed, 206 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json
index 563b4ad..2d45d4c 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -751,6 +751,63 @@
'*service': 'str', '*auth': 'str', '*clients': ['VncClientInfo']} }
##
+# @VncPriAuth:
+#
+# vnc primary authentication method.
+#
+# Since: 2.3
+##
+{ 'enum': 'VncPriAuth',
+ 'data': [ 'none', 'vnc', 'ra2', 'ra2ne', 'tight', 'ultra',
+'tls', 'vencrypt', 'sasl' ] }
+
+##
+# @VncVencryptSubAuth:
+#
+# vnc sub authentication method with vencrypt.
+#
+# Since: 2.3
+##
+{ 'enum': 'VncVencryptSubAuth',
+ 'data': [ 'plain',
+'tls-none', 'x509-none',
+'tls-vnc', 'x509-vnc',
+'tls-plain', 'x509-plain',
+'tls-sasl', 'x509-sasl' ] }
+
+##
+# @VncInfo2:
+#
+# Information about a vnc server
+#
+# @id: vnc server name.
+#
+# @server: A list of @VncBasincInfo describing all listening sockets.
+# The list can be empty (in case the vnc server is disabled).
+# It also may have multiple entries: normal + websocket,
+# possibly also ipv4 + ipv6 in the future.
+#
+# @clients: A list of @VncClientInfo of all currently connected clients.
+# The list can be empty, for obvious reasons.
+#
+# @auth: The current authentication type used by the server
+#
+# @vencrypt: #optional The vencrypt sub authentication type used by the server,
+#only specified in case auth == vencrypt.
+#
+# @display: #optional The display device the vnc server is linked to.
+#
+# Since: 2.3
+##
+{ 'type': 'VncInfo2',
+ 'data': { 'id': 'str',
+'server': ['VncBasicInfo'],
+'clients' : ['VncClientInfo'],
+'auth' : 'VncPriAuth',
+'*vencrypt' : 'VncVencryptSubAuth',
+'*display' : 'str' } }
+
+##
# @query-vnc:
#
# Returns information about the current VNC server
@@ -762,6 +819,17 @@
{ 'command': 'query-vnc', 'returns': 'VncInfo' }
##
+# @query-vnc2:
+#
+# Returns a list of vnc servers. The list can be empty.
+#
+# Returns: @VncInfo
+#
+# Since: 2.3
+##
+{ 'command': 'query-vnc2', 'returns': ['VncInfo2'] }
+
+##
# @SpiceBasicInfo
#
# The basic information for SPICE network connection
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 3348782..dec288a 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2825,6 +2825,11 @@ EQMP
.args_type = "",
.mhandler.cmd_new = qmp_marshal_input_query_vnc,
},
+{
+.name = "query-vnc2",
+.args_type = "",
+.mhandler.cmd_new = qmp_marshal_input_query_vnc2,
+},
SQMP
query-spice
diff --git a/ui/vnc.c b/ui/vnc.c
index d7c7865..e730059 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -456,6 +456,139 @@ out_error:
return NULL;
}
+static VncBasicInfoList *qmp_query_server_entry(int socket,
+VncBasicInfoList *prev)
+{
+VncBasicInfoList *list;
+VncBasicInfo *info;
+struct sockaddr_storage sa;
+socklen_t salen = sizeof(sa);
+char host[NI_MAXHOST];
+char serv[NI_MAXSERV];
+
+if (getsockname(socket, (struct sockaddr *)&sa, &salen) < 0 ||
+getnameinfo((struct sockaddr *)&sa, salen,
+host, sizeof(host), serv, sizeof(serv),
+NI_NUMERICHOST | NI_NUMERICSERV) < 0) {
+return prev;
+}
+
+info = g_new0(VncBasicInfo, 1);
+info->host = g_strdup(host);
+info->service = g_strdup(serv);
+info->family = inet_netfamily(sa.ss_family);
+
+list = g_new0(VncBasicInfoList, 1);
+list->value = info;
+list->next = prev;
+return list;
+}
+
+static void qmp_query_auth(VncDisplay *vd, VncInfo2 *info)
+{
+switch (vd->auth) {
+case VNC_AUTH_VNC:
+info->auth = VNC_PRI_AUTH_VNC;
+break;
+case VNC_AUTH_RA2:
+info->auth = VNC_PRI_AUTH_RA2;
+break;
+case VNC_AUTH_RA2NE:
+info->auth = VNC_PRI_AUTH_RA2NE;
+break;
+case VNC_AUTH_TIGHT:
+info->auth = VNC_PRI_AUTH_TIGHT;
+break;
+case VNC_AUTH_ULTRA:
+info->auth = VNC_PRI_AUTH_ULTRA;
+break;
+case VNC_AUTH_TLS:
+info->auth = VNC_PRI_AUTH_TLS;
+break;
+case VNC_AUTH_VENCRYPT:
+info->auth = VNC_PRI_AUTH_VENCRYPT;
+#ifdef CONFIG_V
[Qemu-devel] [PULL 00/10] vnc: add support for multiple vnc displays
Hi, Ok, here we go with the multiple vnc display patch series. Compared to the v3 series two patch hunks have been moved from patch #9 to patch #2. The final result after applying all patches hasn't changed, so I think it's ok to go straight for a pull req instead of sending a v4. please pull, Gerd The following changes since commit dfa9c2a0f4d0a0c8b2c1449ecdbb1297427e1560: Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2014-12-15 16:43:42 +) are available in the git repository at: git://git.kraxel.org/qemu tags/pull-vnc-20141219-1 for you to fetch changes up to 59235e2be96bbc86e10d8429bda21443b7b9ea57: monitor: add vnc websockets (2014-12-17 15:50:29 +0100) vnc: add support for multiple vnc server instances. Gerd Hoffmann (10): vnc: remove vnc_display global vnc: remove unused DisplayState parameter, add id instead. vnc: add display id to acl names vnc: switch to QemuOpts, allow multiple servers vnc: allow binding servers to qemu consoles vnc: update docs/multiseat.txt vnc: track & limit connections vnc: factor out qmp_query_client_list monitor: add query-vnc2 command monitor: add vnc websockets docs/multiseat.txt | 18 +- include/ui/console.h | 18 +- qapi-schema.json | 73 +- qmp-commands.hx | 5 + qmp.c| 15 +- ui/vnc.c | 634 ++- ui/vnc.h | 5 + vl.c | 41 ++-- 8 files changed, 611 insertions(+), 198 deletions(-)
[Qemu-devel] [PULL 10/10] monitor: add vnc websockets
Add websockets bool to VncBasicInfo, report websocket server sockets,
flag websocket client connections.
Signed-off-by: Gerd Hoffmann
---
qapi-schema.json | 5 -
ui/vnc.c | 15 ---
2 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 2d45d4c..07deb71 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -672,12 +672,15 @@
#
# @family: address family
#
+# @websocket: true in case the socket is a websocket (since 2.3).
+#
# Since: 2.1
##
{ 'type': 'VncBasicInfo',
'data': { 'host': 'str',
'service': 'str',
-'family': 'NetworkAddressFamily' } }
+'family': 'NetworkAddressFamily',
+'websocket': 'bool' } }
##
# @VncServerInfo
diff --git a/ui/vnc.c b/ui/vnc.c
index e730059..fb8068f 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -353,6 +353,9 @@ static VncClientInfo *qmp_query_vnc_client(const VncState
*client)
info->base->host = g_strdup(host);
info->base->service = g_strdup(serv);
info->base->family = inet_netfamily(sa.ss_family);
+#ifdef CONFIG_VNC_WS
+info->base->websocket = client->websocket;
+#endif
#ifdef CONFIG_VNC_TLS
if (client->tls.session && client->tls.dname) {
@@ -457,6 +460,7 @@ out_error:
}
static VncBasicInfoList *qmp_query_server_entry(int socket,
+bool websocket,
VncBasicInfoList *prev)
{
VncBasicInfoList *list;
@@ -477,6 +481,7 @@ static VncBasicInfoList *qmp_query_server_entry(int socket,
info->host = g_strdup(host);
info->service = g_strdup(serv);
info->family = inet_netfamily(sa.ss_family);
+info->websocket = websocket;
list = g_new0(VncBasicInfoList, 1);
list->value = info;
@@ -572,12 +577,13 @@ VncInfo2List *qmp_query_vnc2(Error **errp)
info->display = g_strdup(dev->id);
}
if (vd->lsock != -1) {
-info->server = qmp_query_server_entry(vd->lsock,
+info->server = qmp_query_server_entry(vd->lsock, false,
info->server);
}
#ifdef CONFIG_VNC_WS
if (vd->lwebsock != -1) {
-/* TODO */
+info->server = qmp_query_server_entry(vd->lwebsock, true,
+ info->server);
}
#endif
@@ -3304,10 +3310,13 @@ void vnc_display_open(const char *id, Error **errp)
{
VncDisplay *vs = vnc_display_find(id);
QemuOpts *opts = qemu_opts_find(&qemu_vnc_opts, id);
-const char *display, *websocket, *share, *device_id;
+const char *display, *share, *device_id;
QemuConsole *con;
int password = 0;
int reverse = 0;
+#ifdef CONFIG_VNC_WS
+const char *websocket;
+#endif
#ifdef CONFIG_VNC_TLS
int tls = 0, x509 = 0;
const char *path;
--
1.8.3.1
[Qemu-devel] [PULL 01/10] vnc: remove vnc_display global
Replace with a vnc_displays list, so we can have multiple vnc server
instances. Add vnc_server_find function to lookup a display by id.
With no id supplied return the first vnc server, for backward
compatibility reasons.
It is not possible (yet) to actually create multiple vnc server
instances.
Signed-off-by: Gerd Hoffmann
Reviewed-by: Gonglei
---
ui/vnc.c | 63 +--
ui/vnc.h | 2 ++
2 files changed, 43 insertions(+), 22 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index 5707015..a6549c8 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -46,7 +46,8 @@ static const struct timeval VNC_REFRESH_LOSSY = { 2, 0 };
#include "vnc_keysym.h"
#include "d3des.h"
-static VncDisplay *vnc_display; /* needed for info vnc */
+static QTAILQ_HEAD(, VncDisplay) vnc_displays =
+QTAILQ_HEAD_INITIALIZER(vnc_displays);
static int vnc_cursor_define(VncState *vs);
static void vnc_release_modifiers(VncState *vs);
@@ -226,10 +227,10 @@ static const char *vnc_auth_name(VncDisplay *vd) {
return "unknown";
}
-static VncServerInfo *vnc_server_info_get(void)
+static VncServerInfo *vnc_server_info_get(VncDisplay *vd)
{
VncServerInfo *info;
-VncBasicInfo *bi = vnc_basic_info_get_from_server_addr(vnc_display->lsock);
+VncBasicInfo *bi = vnc_basic_info_get_from_server_addr(vd->lsock);
if (!bi) {
return NULL;
}
@@ -237,7 +238,7 @@ static VncServerInfo *vnc_server_info_get(void)
info = g_malloc(sizeof(*info));
info->base = bi;
info->has_auth = true;
-info->auth = g_strdup(vnc_auth_name(vnc_display));
+info->auth = g_strdup(vnc_auth_name(vd));
return info;
}
@@ -282,7 +283,7 @@ static void vnc_qmp_event(VncState *vs, QAPIEvent event)
}
g_assert(vs->info->base);
-si = vnc_server_info_get();
+si = vnc_server_info_get(vs->vd);
if (!si) {
return;
}
@@ -345,11 +346,27 @@ static VncClientInfo *qmp_query_vnc_client(const VncState
*client)
return info;
}
+static VncDisplay *vnc_display_find(const char *id)
+{
+VncDisplay *vd;
+
+if (id == NULL) {
+return QTAILQ_FIRST(&vnc_displays);
+}
+QTAILQ_FOREACH(vd, &vnc_displays, next) {
+if (strcmp(id, vd->id) == 0) {
+return vd;
+}
+}
+return NULL;
+}
+
VncInfo *qmp_query_vnc(Error **errp)
{
VncInfo *info = g_malloc0(sizeof(*info));
+VncDisplay *vd = vnc_display_find(NULL);
-if (vnc_display == NULL || vnc_display->display == NULL) {
+if (vd == NULL || vd->display == NULL) {
info->enabled = false;
} else {
VncClientInfoList *cur_item = NULL;
@@ -364,7 +381,7 @@ VncInfo *qmp_query_vnc(Error **errp)
/* for compatibility with the original command */
info->has_clients = true;
-QTAILQ_FOREACH(client, &vnc_display->clients, next) {
+QTAILQ_FOREACH(client, &vd->clients, next) {
VncClientInfoList *cinfo = g_malloc0(sizeof(*info));
cinfo->value = qmp_query_vnc_client(client);
@@ -377,11 +394,11 @@ VncInfo *qmp_query_vnc(Error **errp)
}
}
-if (vnc_display->lsock == -1) {
+if (vd->lsock == -1) {
return info;
}
-if (getsockname(vnc_display->lsock, (struct sockaddr *)&sa,
+if (getsockname(vd->lsock, (struct sockaddr *)&sa,
&salen) == -1) {
error_set(errp, QERR_UNDEFINED_ERROR);
goto out_error;
@@ -405,7 +422,7 @@ VncInfo *qmp_query_vnc(Error **errp)
info->family = inet_netfamily(sa.ss_family);
info->has_auth = true;
-info->auth = g_strdup(vnc_auth_name(vnc_display));
+info->auth = g_strdup(vnc_auth_name(vd));
}
return info;
@@ -853,7 +870,7 @@ static int vnc_cursor_define(VncState *vs)
static void vnc_dpy_cursor_define(DisplayChangeListener *dcl,
QEMUCursor *c)
{
-VncDisplay *vd = vnc_display;
+VncDisplay *vd = container_of(dcl, VncDisplay, dcl);
VncState *vs;
cursor_put(vd->cursor);
@@ -2818,6 +2835,7 @@ static void vnc_connect(VncDisplay *vd, int csock,
int i;
vs->csock = csock;
+vs->vd = vd;
if (skipauth) {
vs->auth = VNC_AUTH_NONE;
@@ -2862,8 +2880,6 @@ static void vnc_connect(VncDisplay *vd, int csock,
vnc_qmp_event(vs, QAPI_EVENT_VNC_CONNECTED);
vnc_set_share_mode(vs, VNC_SHARE_MODE_CONNECTING);
-vs->vd = vd;
-
#ifdef CONFIG_VNC_WS
if (!vs->websocket)
#endif
@@ -2955,7 +2971,7 @@ void vnc_display_init(DisplayState *ds)
{
VncDisplay *vs = g_malloc0(sizeof(*vs));
-vnc_display = vs;
+QTAILQ_INSERT_TAIL(&vnc_displays, vs, next);
vs->lsock = -1;
#ifdef CONFIG_VNC_WS
@@ -2985,7 +3001,7 @@ void vnc_display_init(DisplayState *ds)
static void vnc_display_close(DisplayState *ds)
{
-VncDisplay *vs = vnc_display;
+VncDisplay *vs = vnc_display_find(NULL);
[Qemu-devel] [PULL 06/10] vnc: update docs/multiseat.txt
vnc joins the party ;) Also some s/head/seat/ to clarify. Signed-off-by: Gerd Hoffmann --- docs/multiseat.txt | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/docs/multiseat.txt b/docs/multiseat.txt index 67151e0..b963665 100644 --- a/docs/multiseat.txt +++ b/docs/multiseat.txt @@ -7,7 +7,7 @@ host side First you must compile qemu with a user interface supporting multihead/multiseat and input event routing. Right now this -list includes sdl2 and gtk (both 2+3): +list includes sdl2, gtk (both 2+3) and vnc: ./configure --enable-sdl --with-sdlabi=2.0 @@ -16,16 +16,16 @@ or ./configure --enable-gtk -Next put together the qemu command line: +Next put together the qemu command line (sdk/gtk): qemu -enable-kvm -usb $memory $disk $whatever \ -display [ sdl | gtk ] \ -vga std \ -device usb-tablet -That is it for the first head, which will use the standard vga, the +That is it for the first seat, which will use the standard vga, the standard ps/2 keyboard (implicitly there) and the usb-tablet. Now the -additional switches for the second head: +additional switches for the second seat: -device pci-bridge,addr=12.0,chassis_nr=2,id=head.2 \ -device secondary-vga,bus=head.2,addr=02.0,id=video.2 \ @@ -47,6 +47,16 @@ in a separate tab. You can either simply switch tabs to switch heads, or use the "View / Detach tab" menu item to move one of the displays to its own window so you can see both display devices side-by-side. +For vnc some additional configuration on the command line is needed. +We'll create two vnc server instances, and bind the second one to the +second seat, simliar to input devices: + + -display vnc=:1,id=primary \ + -display vnc=:2,id=secondary,display=video.2 + +Connecting to vnc display :1 gives you access to the first seat, and +likewise connecting to vnc display :2 shows the second seat. + Note on spice: Spice handles multihead just fine. But it can't do multiseat. For tablet events the event source is sent to the spice agent. But qemu can't figure it, so it can't do input routing. -- 1.8.3.1
[Qemu-devel] [PULL 03/10] vnc: add display id to acl names
In case the display id is "default" (which is the one you get if you
don't explicitly assign one) we keep the old name scheme, without
display, for backward compatibility reasons.
Signed-off-by: Gerd Hoffmann
Reviewed-by: Gonglei
---
ui/vnc.c | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index fce4861..1b86365 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3206,18 +3206,36 @@ void vnc_display_open(const char *id, const char
*display, Error **errp)
#ifdef CONFIG_VNC_TLS
if (acl && x509 && vs->tls.x509verify) {
-if (!(vs->tls.acl = qemu_acl_init("vnc.x509dname"))) {
+char *aclname;
+
+if (strcmp(vs->id, "default") == 0) {
+aclname = g_strdup("vnc.x509dname");
+} else {
+aclname = g_strdup_printf("vnc.%s.x509dname", vs->id);
+}
+vs->tls.acl = qemu_acl_init(aclname);
+if (!vs->tls.acl) {
fprintf(stderr, "Failed to create x509 dname ACL\n");
exit(1);
}
+g_free(aclname);
}
#endif
#ifdef CONFIG_VNC_SASL
if (acl && sasl) {
-if (!(vs->sasl.acl = qemu_acl_init("vnc.username"))) {
+char *aclname;
+
+if (strcmp(vs->id, "default") == 0) {
+aclname = g_strdup("vnc.username");
+} else {
+aclname = g_strdup_printf("vnc.%s.username", vs->id);
+}
+vs->sasl.acl = qemu_acl_init(aclname);
+if (!vs->sasl.acl) {
fprintf(stderr, "Failed to create username ACL\n");
exit(1);
}
+g_free(aclname);
}
#endif
--
1.8.3.1
[Qemu-devel] [PULL 04/10] vnc: switch to QemuOpts, allow multiple servers
This patch switches vnc over to QemuOpts, and it (more or less
as side effect) allows multiple vnc server instances.
Signed-off-by: Gerd Hoffmann
---
include/ui/console.h | 4 +-
qmp.c| 15 ++-
ui/vnc.c | 270 ---
vl.c | 42 +++-
4 files changed, 199 insertions(+), 132 deletions(-)
diff --git a/include/ui/console.h b/include/ui/console.h
index 5ff2e27..887ed91 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -328,12 +328,14 @@ void cocoa_display_init(DisplayState *ds, int
full_screen);
/* vnc.c */
void vnc_display_init(const char *id);
-void vnc_display_open(const char *id, const char *display, Error **errp);
+void vnc_display_open(const char *id, Error **errp);
void vnc_display_add_client(const char *id, int csock, bool skipauth);
char *vnc_display_local_addr(const char *id);
#ifdef CONFIG_VNC
int vnc_display_password(const char *id, const char *password);
int vnc_display_pw_expire(const char *id, time_t expires);
+QemuOpts *vnc_parse_func(const char *str);
+int vnc_init_func(QemuOpts *opts, void *opaque);
#else
static inline int vnc_display_password(const char *id, const char *password)
{
diff --git a/qmp.c b/qmp.c
index 0b4f131..963305c 100644
--- a/qmp.c
+++ b/qmp.c
@@ -368,7 +368,20 @@ void qmp_change_vnc_password(const char *password, Error
**errp)
static void qmp_change_vnc_listen(const char *target, Error **errp)
{
-vnc_display_open(NULL, target, errp);
+QemuOptsList *olist = qemu_find_opts("vnc");
+QemuOpts *opts;
+
+if (strstr(target, "id=")) {
+error_setg(errp, "id not supported");
+return;
+}
+
+opts = qemu_opts_find(olist, "default");
+if (opts) {
+qemu_opts_del(opts);
+}
+opts = vnc_parse_func(target);
+vnc_display_open("default", errp);
}
static void qmp_change_vnc(const char *target, bool has_arg, const char *arg,
diff --git a/ui/vnc.c b/ui/vnc.c
index 1b86365..ce1dd59 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -31,6 +31,7 @@
#include "qemu/sockets.h"
#include "qemu/timer.h"
#include "qemu/acl.h"
+#include "qemu/config-file.h"
#include "qapi/qmp/types.h"
#include "qmp-commands.h"
#include "qemu/osdep.h"
@@ -2969,7 +2970,12 @@ static const DisplayChangeListenerOps dcl_ops = {
void vnc_display_init(const char *id)
{
-VncDisplay *vs = g_malloc0(sizeof(*vs));
+VncDisplay *vs;
+
+if (vnc_display_find(id) != NULL) {
+return;
+}
+vs = g_malloc0(sizeof(*vs));
vs->id = strdup(id);
QTAILQ_INSERT_TAIL(&vnc_displays, vs, next);
@@ -3065,14 +3071,65 @@ char *vnc_display_local_addr(const char *id)
return vnc_socket_local_addr("%s:%s", vs->lsock);
}
-void vnc_display_open(const char *id, const char *display, Error **errp)
+static QemuOptsList qemu_vnc_opts = {
+.name = "vnc",
+.head = QTAILQ_HEAD_INITIALIZER(qemu_vnc_opts.head),
+.implied_opt_name = "vnc",
+.desc = {
+{
+.name = "vnc",
+.type = QEMU_OPT_STRING,
+},{
+.name = "websocket",
+.type = QEMU_OPT_STRING,
+},{
+.name = "x509",
+.type = QEMU_OPT_STRING,
+},{
+.name = "share",
+.type = QEMU_OPT_STRING,
+},{
+.name = "password",
+.type = QEMU_OPT_BOOL,
+},{
+.name = "reverse",
+.type = QEMU_OPT_BOOL,
+},{
+.name = "lock-key-sync",
+.type = QEMU_OPT_BOOL,
+},{
+.name = "sasl",
+.type = QEMU_OPT_BOOL,
+},{
+.name = "tls",
+.type = QEMU_OPT_BOOL,
+},{
+.name = "x509verify",
+.type = QEMU_OPT_BOOL,
+},{
+.name = "acl",
+.type = QEMU_OPT_BOOL,
+},{
+.name = "lossy",
+.type = QEMU_OPT_BOOL,
+},{
+.name = "non-adaptive",
+.type = QEMU_OPT_BOOL,
+},
+{ /* end of list */ }
+},
+};
+
+void vnc_display_open(const char *id, Error **errp)
{
VncDisplay *vs = vnc_display_find(id);
-const char *options;
+QemuOpts *opts = qemu_opts_find(&qemu_vnc_opts, id);
+const char *display, *websocket, *share;
int password = 0;
int reverse = 0;
#ifdef CONFIG_VNC_TLS
int tls = 0, x509 = 0;
+const char *path;
#endif
#ifdef CONFIG_VNC_SASL
int sasl = 0;
@@ -3088,115 +3145,86 @@ void vnc_display_open(const char *id, const char
*display, Error **errp)
return;
}
vnc_display_close(vs);
-if (strcmp(display, "none") == 0)
-return;
+if (!opts) {
+return;
+}
+display = qemu_opt_get(opts, "vnc");
+if (!display || strcmp(display, "none") == 0) {
+return;
+}
vs->display = g_strdup(display);
-vs->share_policy = VNC_SHARE_POLICY_ALLOW_EXCLUSIVE;
-
-options = display;
-
[Qemu-devel] [PULL 05/10] vnc: allow binding servers to qemu consoles
This patch adds a display= parameter to the vnc options. This allows to
bind a vnc server instance to a specific display, allowing to create a
multiseat setup with a vnc server for each seat.
Signed-off-by: Gerd Hoffmann
---
ui/vnc.c | 50 +++---
1 file changed, 43 insertions(+), 7 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index ce1dd59..dd09fc1 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -27,6 +27,7 @@
#include "vnc.h"
#include "vnc-jobs.h"
#include "trace.h"
+#include "hw/qdev.h"
#include "sysemu/sysemu.h"
#include "qemu/sockets.h"
#include "qemu/timer.h"
@@ -1665,7 +1666,8 @@ static void do_key_event(VncState *vs, int down, int
keycode, int sym)
vs->modifiers_state[keycode] = 0;
break;
case 0x02 ... 0x0a: /* '1' to '9' keys */
-if (down && vs->modifiers_state[0x1d] && vs->modifiers_state[0x38]) {
+if (vs->vd->dcl.con == NULL &&
+down && vs->modifiers_state[0x1d] && vs->modifiers_state[0x38]) {
/* Reset the modifiers sent to the current console */
reset_keys(vs);
console_select(keycode - 0x02);
@@ -2073,8 +2075,8 @@ static void set_pixel_format(VncState *vs,
set_pixel_conversion(vs);
-graphic_hw_invalidate(NULL);
-graphic_hw_update(NULL);
+graphic_hw_invalidate(vs->vd->dcl.con);
+graphic_hw_update(vs->vd->dcl.con);
}
static void pixel_format_message (VncState *vs) {
@@ -2801,7 +2803,7 @@ static void vnc_refresh(DisplayChangeListener *dcl)
return;
}
-graphic_hw_update(NULL);
+graphic_hw_update(vd->dcl.con);
if (vnc_trylock_display(vd)) {
update_displaychangelistener(&vd->dcl, VNC_REFRESH_INTERVAL_BASE);
@@ -2907,7 +2909,7 @@ void vnc_init_state(VncState *vs)
QTAILQ_INSERT_HEAD(&vd->clients, vs, next);
-graphic_hw_update(NULL);
+graphic_hw_update(vd->dcl.con);
vnc_write(vs, "RFB 003.008\n", 12);
vnc_flush(vs);
@@ -2930,7 +2932,7 @@ static void vnc_listen_read(void *opaque, bool websocket)
int csock;
/* Catch-up */
-graphic_hw_update(NULL);
+graphic_hw_update(vs->dcl.con);
#ifdef CONFIG_VNC_WS
if (websocket) {
csock = qemu_accept(vs->lwebsock, (struct sockaddr *)&addr, &addrlen);
@@ -3089,6 +3091,12 @@ static QemuOptsList qemu_vnc_opts = {
.name = "share",
.type = QEMU_OPT_STRING,
},{
+.name = "display",
+.type = QEMU_OPT_STRING,
+},{
+.name = "head",
+.type = QEMU_OPT_NUMBER,
+},{
.name = "password",
.type = QEMU_OPT_BOOL,
},{
@@ -3124,7 +3132,8 @@ void vnc_display_open(const char *id, Error **errp)
{
VncDisplay *vs = vnc_display_find(id);
QemuOpts *opts = qemu_opts_find(&qemu_vnc_opts, id);
-const char *display, *websocket, *share;
+const char *display, *websocket, *share, *device_id;
+QemuConsole *con;
int password = 0;
int reverse = 0;
#ifdef CONFIG_VNC_TLS
@@ -3353,6 +3362,33 @@ void vnc_display_open(const char *id, Error **errp)
#endif
vs->lock_key_sync = lock_key_sync;
+device_id = qemu_opt_get(opts, "display");
+if (device_id) {
+DeviceState *dev;
+int head = qemu_opt_get_number(opts, "head", 0);
+
+dev = qdev_find_recursive(sysbus_get_default(), device_id);
+if (dev == NULL) {
+error_set(errp, QERR_DEVICE_NOT_FOUND, device_id);
+goto fail;
+}
+
+con = qemu_console_lookup_by_device(dev, head);
+if (con == NULL) {
+error_setg(errp, "Device %s is not bound to a QemuConsole",
+ device_id);
+goto fail;
+}
+} else {
+con = NULL;
+}
+
+if (con != vs->dcl.con) {
+unregister_displaychangelistener(&vs->dcl);
+vs->dcl.con = con;
+register_displaychangelistener(&vs->dcl);
+}
+
if (reverse) {
/* connect to viewer */
int csock;
--
1.8.3.1
[Qemu-devel] [PULL 07/10] vnc: track & limit connections
Also track the number of connections in "connecting" and "shared" state
(in addition to the "exclusive" state). Apply a configurable limit to
these connections.
The logic to apply the limit to connections in "shared" state is pretty
simple: When the limit is reached no new connections are allowed.
The logic to apply the limit to connections in "connecting" state (this
is the state you are in *before* successful authentication) is
slightly different: A new connect kicks out the oldest client which is
still in "connecting" state. This avoids a easy DoS by unauthenticated
users by simply opening connections until the limit is reached.
Cc: Dr. David Alan Gilbert
Signed-off-by: Gerd Hoffmann
---
ui/vnc.c | 46 +++---
ui/vnc.h | 3 +++
2 files changed, 46 insertions(+), 3 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index dd09fc1..2ed16dc 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -68,12 +68,34 @@ static void vnc_set_share_mode(VncState *vs, VncShareMode
mode)
vs->csock, mn[vs->share_mode], mn[mode]);
#endif
-if (vs->share_mode == VNC_SHARE_MODE_EXCLUSIVE) {
+switch (vs->share_mode) {
+case VNC_SHARE_MODE_CONNECTING:
+vs->vd->num_connecting--;
+break;
+case VNC_SHARE_MODE_SHARED:
+vs->vd->num_shared--;
+break;
+case VNC_SHARE_MODE_EXCLUSIVE:
vs->vd->num_exclusive--;
+break;
+default:
+break;
}
+
vs->share_mode = mode;
-if (vs->share_mode == VNC_SHARE_MODE_EXCLUSIVE) {
+
+switch (vs->share_mode) {
+case VNC_SHARE_MODE_CONNECTING:
+vs->vd->num_connecting++;
+break;
+case VNC_SHARE_MODE_SHARED:
+vs->vd->num_shared++;
+break;
+case VNC_SHARE_MODE_EXCLUSIVE:
vs->vd->num_exclusive++;
+break;
+default:
+break;
}
}
@@ -2337,6 +2359,11 @@ static int protocol_client_init(VncState *vs, uint8_t
*data, size_t len)
}
vnc_set_share_mode(vs, mode);
+if (vs->vd->num_shared > vs->vd->connections_limit) {
+vnc_disconnect_start(vs);
+return 0;
+}
+
vs->client_width = pixman_image_get_width(vs->vd->server);
vs->client_height = pixman_image_get_height(vs->vd->server);
vnc_write_u16(vs, vs->client_width);
@@ -2889,6 +2916,15 @@ static void vnc_connect(VncDisplay *vd, int csock,
{
vnc_init_state(vs);
}
+
+if (vd->num_connecting > vd->connections_limit) {
+QTAILQ_FOREACH(vs, &vd->clients, next) {
+if (vs->share_mode == VNC_SHARE_MODE_CONNECTING) {
+vnc_disconnect_start(vs);
+return;
+}
+}
+}
}
void vnc_init_state(VncState *vs)
@@ -2907,7 +2943,7 @@ void vnc_init_state(VncState *vs)
qemu_mutex_init(&vs->output_mutex);
vs->bh = qemu_bh_new(vnc_jobs_bh, vs);
-QTAILQ_INSERT_HEAD(&vd->clients, vs, next);
+QTAILQ_INSERT_TAIL(&vd->clients, vs, next);
graphic_hw_update(vd->dcl.con);
@@ -3097,6 +3133,9 @@ static QemuOptsList qemu_vnc_opts = {
.name = "head",
.type = QEMU_OPT_NUMBER,
},{
+.name = "connections",
+.type = QEMU_OPT_NUMBER,
+},{
.name = "password",
.type = QEMU_OPT_BOOL,
},{
@@ -3210,6 +3249,7 @@ void vnc_display_open(const char *id, Error **errp)
} else {
vs->share_policy = VNC_SHARE_POLICY_ALLOW_EXCLUSIVE;
}
+vs->connections_limit = qemu_opt_get_number(opts, "connections", 32);
#ifdef CONFIG_VNC_WS
websocket = qemu_opt_get(opts, "websocket");
diff --git a/ui/vnc.h b/ui/vnc.h
index 6fe8278..5e2b1a5 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -150,7 +150,10 @@ typedef enum VncSharePolicy {
struct VncDisplay
{
QTAILQ_HEAD(, VncState) clients;
+int num_connecting;
+int num_shared;
int num_exclusive;
+int connections_limit;
VncSharePolicy share_policy;
int lsock;
#ifdef CONFIG_VNC_WS
--
1.8.3.1
[Qemu-devel] [PULL 08/10] vnc: factor out qmp_query_client_list
so we can reuse it for the new vnc query command.
Signed-off-by: Gerd Hoffmann
---
ui/vnc.c | 30 +++---
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index 2ed16dc..d7c7865 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -385,6 +385,20 @@ static VncDisplay *vnc_display_find(const char *id)
return NULL;
}
+static VncClientInfoList *qmp_query_client_list(VncDisplay *vd)
+{
+VncClientInfoList *cinfo, *prev = NULL;
+VncState *client;
+
+QTAILQ_FOREACH(client, &vd->clients, next) {
+cinfo = g_new0(VncClientInfoList, 1);
+cinfo->value = qmp_query_vnc_client(client);
+cinfo->next = prev;
+prev = cinfo;
+}
+return prev;
+}
+
VncInfo *qmp_query_vnc(Error **errp)
{
VncInfo *info = g_malloc0(sizeof(*info));
@@ -393,30 +407,16 @@ VncInfo *qmp_query_vnc(Error **errp)
if (vd == NULL || vd->display == NULL) {
info->enabled = false;
} else {
-VncClientInfoList *cur_item = NULL;
struct sockaddr_storage sa;
socklen_t salen = sizeof(sa);
char host[NI_MAXHOST];
char serv[NI_MAXSERV];
-VncState *client;
info->enabled = true;
/* for compatibility with the original command */
info->has_clients = true;
-
-QTAILQ_FOREACH(client, &vd->clients, next) {
-VncClientInfoList *cinfo = g_malloc0(sizeof(*info));
-cinfo->value = qmp_query_vnc_client(client);
-
-/* XXX: waiting for the qapi to support GSList */
-if (!cur_item) {
-info->clients = cur_item = cinfo;
-} else {
-cur_item->next = cinfo;
-cur_item = cinfo;
-}
-}
+info->clients = qmp_query_client_list(vd);
if (vd->lsock == -1) {
return info;
--
1.8.3.1
Re: [Qemu-devel] [PATCH V3] net: don't use set/get_pointer() in set/get_netdev()
This one fell through the cracks, sorry.
Jason Wang writes:
> Commit 1ceef9f27359cbe92ef124bf74de6f792e71f6fb (net: multiqueue
> support) tries to use set_pointer() and get_pointer() to set and get
> NICPeers which is not a pointer defined in DEFINE_PROP_NETDEV. This
> trick works but result a unclean and fragile implementation (e.g
> print_netdev and parse_netdev).
>
> This patch solves this issue by not using set/get_pinter() and set and
> get netdev directly in set_netdev() and get_netdev(). After this the
> parse_netdev() and print_netdev() were no longer used and dropped from
> the source.
>
> Cc: Markus Armbruster
> Cc: Stefan Hajnoczi
> Cc: Peter Maydell
> Signed-off-by: Jason Wang
> ---
> Changes from V2:
> - Use error_setg() instead of error_set_from_qdev_prop_error() for E2BIG
> error.
> - Clean the return part of the set_netdev() since
> eror_set_from_qdev_prop_error() does nothing when err is 0.
> Changes from V1:
> - validate ncs pointer before accessing them, this fixes the qtest failure
> on arm.
> ---
> hw/core/qdev-properties-system.c | 70
> ++--
> 1 file changed, 38 insertions(+), 32 deletions(-)
>
> diff --git a/hw/core/qdev-properties-system.c
> b/hw/core/qdev-properties-system.c
> index 84caa1d..03a6e68 100644
> --- a/hw/core/qdev-properties-system.c
> +++ b/hw/core/qdev-properties-system.c
> @@ -176,41 +176,68 @@ PropertyInfo qdev_prop_chr = {
> };
>
> /* --- netdev device --- */
> +static void get_netdev(Object *obj, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> +DeviceState *dev = DEVICE(obj);
> +Property *prop = opaque;
> +NICPeers *peers_ptr = qdev_get_prop_ptr(dev, prop);
> +char *p = g_strdup(peers_ptr->ncs[0] ? peers_ptr->ncs[0]->name : "");
>
> -static int parse_netdev(DeviceState *dev, const char *str, void **ptr)
> +visit_type_str(v, &p, name, errp);
> +g_free(p);
> +}
> +
> +static void set_netdev(Object *obj, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> {
> -NICPeers *peers_ptr = (NICPeers *)ptr;
> +DeviceState *dev = DEVICE(obj);
> +Property *prop = opaque;
> +NICPeers *peers_ptr = qdev_get_prop_ptr(dev, prop);
> NetClientState **ncs = peers_ptr->ncs;
> NetClientState *peers[MAX_QUEUE_NUM];
> -int queues, i = 0;
> -int ret;
> +Error *local_err = NULL;
> +int queues, err = 0, i = 0;
> +char *str;
> +
> +if (dev->realized) {
> +qdev_prop_set_after_realize(dev, name, errp);
> +return;
> +}
> +
> +visit_type_str(v, &str, name, &local_err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
>
> queues = qemu_find_net_clients_except(str, peers,
>NET_CLIENT_OPTIONS_KIND_NIC,
>MAX_QUEUE_NUM);
> if (queues == 0) {
> -ret = -ENOENT;
> +err = -ENOENT;
> goto err;
> }
>
> if (queues > MAX_QUEUE_NUM) {
> -ret = -E2BIG;
> +error_setg(errp, "queues of backend '%s'(%d) exceeds QEMU
> limitation(%d)",
> + str, queues, MAX_QUEUE_NUM);
> goto err;
> }
>
> for (i = 0; i < queues; i++) {
> if (peers[i] == NULL) {
> -ret = -ENOENT;
> +err = -ENOENT;
> goto err;
> }
>
> if (peers[i]->peer) {
> -ret = -EEXIST;
> +err = -EEXIST;
> goto err;
> }
>
> if (ncs[i]) {
> -ret = -EINVAL;
> +err = -EINVAL;
> goto err;
> }
>
> @@ -220,30 +247,9 @@ static int parse_netdev(DeviceState *dev, const char
> *str, void **ptr)
>
> peers_ptr->queues = queues;
>
> -return 0;
> -
> err:
Label err isn't the error path, it's the common path. It also clashes
with local variable err. Both harmless, but perhaps it could be renamed
to out on commit.
> -return ret;
> -}
> -
> -static char *print_netdev(void *ptr)
> -{
> -NetClientState *netdev = ptr;
> -const char *val = netdev->name ? netdev->name : "";
> -
> -return g_strdup(val);
> -}
> -
> -static void get_netdev(Object *obj, Visitor *v, void *opaque,
> - const char *name, Error **errp)
> -{
> -get_pointer(obj, v, opaque, print_netdev, name, errp);
> -}
> -
> -static void set_netdev(Object *obj, Visitor *v, void *opaque,
> - const char *name, Error **errp)
> -{
> -set_pointer(obj, v, opaque, parse_netdev, name, errp);
> +error_set_from_qdev_prop_error(errp, err, dev, prop, str);
> +g_free(str);
> }
>
> PropertyInfo qdev_prop_netdev = {
Reviewed-by: Markus Armbruster
[Qemu-devel] [PULL 06/12] sdl2: overhaul window size handling
Split do_sdl_resize function (which does alot more than just resizing)
into three: sdl2_window_{create,destroy,resize}.
Fix SDL_Renderer handling: must be guest display size not host window
size, and SDL2 will magically handle all scaling for us.
Make fullscreen actually enter fullscreen mode and simplify the code.
There is no need to store the original window size, the window manager
will do that for us.
Signed-off-by: Gerd Hoffmann
Reviewed-by: Max Reitz
---
ui/sdl2.c | 172 +-
1 file changed, 79 insertions(+), 93 deletions(-)
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 47a757a..59b67a6 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -41,9 +41,6 @@ static struct sdl2_console *sdl2_console;
static SDL_Surface *guest_sprite_surface;
static int gui_grab; /* if true, all keyboard/mouse events are grabbed */
-static bool gui_saved_scaling;
-static int gui_saved_width;
-static int gui_saved_height;
static int gui_saved_grab;
static int gui_fullscreen;
static int gui_noframe;
@@ -56,7 +53,6 @@ static int absolute_enabled;
static int guest_cursor;
static int guest_x, guest_y;
static SDL_Cursor *guest_sprite;
-static int scaling_active;
static Notifier mouse_mode_notifier;
static void sdl_update_caption(struct sdl2_console *scon);
@@ -72,86 +68,96 @@ static struct sdl2_console *get_scon_from_window(uint32_t
window_id)
return NULL;
}
-static void do_sdl_resize(struct sdl2_console *scon, int width, int height,
- int bpp)
+static void sdl2_window_create(struct sdl2_console *scon)
{
-int flags;
+int flags = 0;
-if (scon->real_window && scon->real_renderer) {
-if (width && height) {
-SDL_RenderSetLogicalSize(scon->real_renderer, width, height);
-SDL_SetWindowSize(scon->real_window, width, height);
-} else {
-SDL_DestroyRenderer(scon->real_renderer);
-SDL_DestroyWindow(scon->real_window);
-scon->real_renderer = NULL;
-scon->real_window = NULL;
-}
+if (!scon->surface) {
+return;
+}
+assert(!scon->real_window);
+
+if (gui_fullscreen) {
+flags |= SDL_WINDOW_FULLSCREEN_DESKTOP;
} else {
-if (!width || !height) {
-return;
-}
-flags = 0;
-if (gui_fullscreen) {
-flags |= SDL_WINDOW_FULLSCREEN;
-} else {
-flags |= SDL_WINDOW_RESIZABLE;
-}
-if (scon->hidden) {
-flags |= SDL_WINDOW_HIDDEN;
-}
+flags |= SDL_WINDOW_RESIZABLE;
+}
+if (scon->hidden) {
+flags |= SDL_WINDOW_HIDDEN;
+}
-scon->real_window = SDL_CreateWindow("", SDL_WINDOWPOS_UNDEFINED,
- SDL_WINDOWPOS_UNDEFINED,
- width, height, flags);
-scon->real_renderer = SDL_CreateRenderer(scon->real_window, -1, 0);
-sdl_update_caption(scon);
+scon->real_window = SDL_CreateWindow("", SDL_WINDOWPOS_UNDEFINED,
+ SDL_WINDOWPOS_UNDEFINED,
+ surface_width(scon->surface),
+ surface_height(scon->surface),
+ flags);
+scon->real_renderer = SDL_CreateRenderer(scon->real_window, -1, 0);
+sdl_update_caption(scon);
+}
+
+static void sdl2_window_destroy(struct sdl2_console *scon)
+{
+if (!scon->real_window) {
+return;
}
+
+SDL_DestroyRenderer(scon->real_renderer);
+scon->real_renderer = NULL;
+SDL_DestroyWindow(scon->real_window);
+scon->real_window = NULL;
+}
+
+static void sdl2_window_resize(struct sdl2_console *scon)
+{
+if (!scon->real_window) {
+return;
+}
+
+SDL_SetWindowSize(scon->real_window,
+ surface_width(scon->surface),
+ surface_height(scon->surface));
}
static void sdl_switch(DisplayChangeListener *dcl,
DisplaySurface *new_surface)
{
struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
-int format = 0;
-int idx = scon->idx;
DisplaySurface *old_surface = scon->surface;
+int format = 0;
-/* temporary hack: allows to call sdl_switch to handle scaling changes */
-if (new_surface) {
-scon->surface = new_surface;
-}
+scon->surface = new_surface;
-if (!new_surface && idx > 0) {
-scon->surface = NULL;
+if (scon->texture) {
+SDL_DestroyTexture(scon->texture);
+scon->texture = NULL;
}
-if (new_surface == NULL) {
-do_sdl_resize(scon, 0, 0, 0);
-} else {
-do_sdl_resize(scon, surface_width(scon->surface),
- surface_height(scon->surface), 0);
+if (!new_surface) {
+sdl2_window_destroy(scon);
+return;
}
-if (old_surface && scon
[Qemu-devel] [PULL 02/12] sdl2: rename sdl2_state to sdl2_console, move to header file
Create sdl2.h header file, in preparation for sdl2 code splitup.
Populate it with sdl2_console struct (renamed from sdl2_state).
Signed-off-by: Gerd Hoffmann
Reviewed-by: Max Reitz
---
include/ui/sdl2.h | 16 ++
ui/sdl2.c | 63 ---
2 files changed, 43 insertions(+), 36 deletions(-)
create mode 100644 include/ui/sdl2.h
diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
new file mode 100644
index 000..3ad5d7d
--- /dev/null
+++ b/include/ui/sdl2.h
@@ -0,0 +1,16 @@
+#ifndef SDL2_H
+#define SDL2_H
+
+struct sdl2_console {
+DisplayChangeListener dcl;
+DisplaySurface *surface;
+SDL_Texture *texture;
+SDL_Window *real_window;
+SDL_Renderer *real_renderer;
+int idx;
+int last_vm_running; /* per console for caption reasons */
+int x, y;
+int hidden;
+};
+
+#endif /* SDL2_H */
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 45f23b1..375e1a3 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -32,22 +32,13 @@
#include "qemu-common.h"
#include "ui/console.h"
#include "ui/input.h"
+#include "ui/sdl2.h"
#include "sysemu/sysemu.h"
#include "sdl2-keymap.h"
static int sdl2_num_outputs;
-static struct sdl2_state {
-DisplayChangeListener dcl;
-DisplaySurface *surface;
-SDL_Texture *texture;
-SDL_Window *real_window;
-SDL_Renderer *real_renderer;
-int idx;
-int last_vm_running; /* per console for caption reasons */
-int x, y;
-int hidden;
-} *sdl2_console;
+static struct sdl2_console *sdl2_console;
static SDL_Surface *guest_sprite_surface;
static int gui_grab; /* if true, all keyboard/mouse events are grabbed */
@@ -71,9 +62,9 @@ static SDL_Cursor *guest_sprite;
static int scaling_active;
static Notifier mouse_mode_notifier;
-static void sdl_update_caption(struct sdl2_state *scon);
+static void sdl_update_caption(struct sdl2_console *scon);
-static struct sdl2_state *get_scon_from_window(uint32_t window_id)
+static struct sdl2_console *get_scon_from_window(uint32_t window_id)
{
int i;
for (i = 0; i < sdl2_num_outputs; i++) {
@@ -87,7 +78,7 @@ static struct sdl2_state *get_scon_from_window(uint32_t
window_id)
static void sdl_update(DisplayChangeListener *dcl,
int x, int y, int w, int h)
{
-struct sdl2_state *scon = container_of(dcl, struct sdl2_state, dcl);
+struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
SDL_Rect rect;
DisplaySurface *surf = qemu_console_surface(dcl->con);
@@ -109,7 +100,7 @@ static void sdl_update(DisplayChangeListener *dcl,
SDL_RenderPresent(scon->real_renderer);
}
-static void do_sdl_resize(struct sdl2_state *scon, int width, int height,
+static void do_sdl_resize(struct sdl2_console *scon, int width, int height,
int bpp)
{
int flags;
@@ -149,7 +140,7 @@ static void do_sdl_resize(struct sdl2_state *scon, int
width, int height,
static void sdl_switch(DisplayChangeListener *dcl,
DisplaySurface *new_surface)
{
-struct sdl2_state *scon = container_of(dcl, struct sdl2_state, dcl);
+struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
int format = 0;
int idx = scon->idx;
DisplaySurface *old_surface = scon->surface;
@@ -191,7 +182,7 @@ static void sdl_switch(DisplayChangeListener *dcl,
}
}
-static void reset_keys(struct sdl2_state *scon)
+static void reset_keys(struct sdl2_console *scon)
{
QemuConsole *con = scon ? scon->dcl.con : NULL;
int i;
@@ -205,7 +196,7 @@ static void reset_keys(struct sdl2_state *scon)
}
}
-static void sdl_process_key(struct sdl2_state *scon,
+static void sdl_process_key(struct sdl2_console *scon,
SDL_KeyboardEvent *ev)
{
int qcode = sdl2_scancode_to_qcode[ev->keysym.scancode];
@@ -257,7 +248,7 @@ static void sdl_process_key(struct sdl2_state *scon,
}
}
-static void sdl_update_caption(struct sdl2_state *scon)
+static void sdl_update_caption(struct sdl2_console *scon)
{
char win_title[1024];
char icon_title[1024];
@@ -321,7 +312,7 @@ static void sdl_show_cursor(void)
}
}
-static void sdl_grab_start(struct sdl2_state *scon)
+static void sdl_grab_start(struct sdl2_console *scon)
{
QemuConsole *con = scon ? scon->dcl.con : NULL;
@@ -349,7 +340,7 @@ static void sdl_grab_start(struct sdl2_state *scon)
sdl_update_caption(scon);
}
-static void sdl_grab_end(struct sdl2_state *scon)
+static void sdl_grab_end(struct sdl2_console *scon)
{
SDL_SetWindowGrab(scon->real_window, SDL_FALSE);
gui_grab = 0;
@@ -357,7 +348,7 @@ static void sdl_grab_end(struct sdl2_state *scon)
sdl_update_caption(scon);
}
-static void absolute_mouse_grab(struct sdl2_state *scon)
+static void absolute_mouse_grab(struct sdl2_console *scon)
{
int mouse_x, mouse_y;
int scr_w, scr_h;
@@ -384,7 +375,7 @@ static void sdl_mouse_mode_change(Notifier *notify,
[Qemu-devel] [PULL 00/12] sdl2: fixes, cleanups and opengl preparation.
Hi, So, here we go with the sdl2 pull request, no changes to v2 patches series other than adding the two fixes from Max. please pull, Gerd The following changes since commit dfa9c2a0f4d0a0c8b2c1449ecdbb1297427e1560: Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2014-12-15 16:43:42 +) are available in the git repository at: git://git.kraxel.org/qemu tags/pull-sdl-20141219-1 for you to fetch changes up to d3f3a0f453ea590be529079ae214c200bb5ecc1a: sdl2: Work around SDL2 SDL_ShowWindow() bug (2014-12-17 12:44:01 +0100) sdl2: fixes, cleanups and opengl preparation. Gerd Hoffmann (10): sdl: move version logic from source code to makefile sdl2: rename sdl2_state to sdl2_console, move to header file sdl2: move keyboard input code to new sdl2-input.c sdl2: turn on keyboard grabs sdl2: move sdl_update to new sdl2-2d.c sdl2: overhaul window size handling sdl2: move sdl_switch to sdl2-2d.c sdl2: add+use sdl2_2d_redraw function. sdl2: factor out sdl2_poll_events sdl2: move sdl2_2d_refresh to sdl2-2d.c Max Reitz (2): sdl2: Use correct sdl2_console for window events sdl2: Work around SDL2 SDL_ShowWindow() bug include/ui/sdl2.h | 32 + ui/Makefile.objs | 7 +- ui/sdl.c | 3 - ui/sdl2-2d.c | 122 +++ ui/sdl2-input.c | 106 + ui/sdl2.c | 343 +++--- 6 files changed, 358 insertions(+), 255 deletions(-) create mode 100644 include/ui/sdl2.h create mode 100644 ui/sdl2-2d.c create mode 100644 ui/sdl2-input.c
[Qemu-devel] [PULL 03/12] sdl2: move keyboard input code to new sdl2-input.c
Signed-off-by: Gerd Hoffmann
Reviewed-by: Max Reitz
---
include/ui/sdl2.h | 4 +++
ui/Makefile.objs | 2 +-
ui/sdl2-input.c | 106 ++
ui/sdl2.c | 75 ++
4 files changed, 114 insertions(+), 73 deletions(-)
create mode 100644 ui/sdl2-input.c
diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
index 3ad5d7d..77d800e 100644
--- a/include/ui/sdl2.h
+++ b/include/ui/sdl2.h
@@ -13,4 +13,8 @@ struct sdl2_console {
int hidden;
};
+void sdl2_reset_keys(struct sdl2_console *scon);
+void sdl2_process_key(struct sdl2_console *scon,
+ SDL_KeyboardEvent *ev);
+
#endif /* SDL2_H */
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index b25e85f..011c5bb 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -20,7 +20,7 @@ ifeq ($(CONFIG_SDLABI),1.2)
sdl.mo-objs := sdl.o sdl_zoom.o
endif
ifeq ($(CONFIG_SDLABI),2.0)
-sdl.mo-objs := sdl2.o
+sdl.mo-objs := sdl2.o sdl2-input.o
endif
sdl.mo-cflags := $(SDL_CFLAGS)
diff --git a/ui/sdl2-input.c b/ui/sdl2-input.c
new file mode 100644
index 000..a1973fc
--- /dev/null
+++ b/ui/sdl2-input.c
@@ -0,0 +1,106 @@
+/*
+ * QEMU SDL display driver
+ *
+ * Copyright (c) 2003 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+/* Ported SDL 1.2 code to 2.0 by Dave Airlie. */
+
+/* Avoid compiler warning because macro is redefined in SDL_syswm.h. */
+#undef WIN32_LEAN_AND_MEAN
+
+#include
+#include
+
+#include "qemu-common.h"
+#include "ui/console.h"
+#include "ui/input.h"
+#include "ui/sdl2.h"
+#include "sysemu/sysemu.h"
+
+#include "sdl2-keymap.h"
+
+static uint8_t modifiers_state[SDL_NUM_SCANCODES];
+
+void sdl2_reset_keys(struct sdl2_console *scon)
+{
+QemuConsole *con = scon ? scon->dcl.con : NULL;
+int i;
+
+for (i = 0; i < SDL_NUM_SCANCODES; i++) {
+if (modifiers_state[i]) {
+int qcode = sdl2_scancode_to_qcode[i];
+qemu_input_event_send_key_qcode(con, qcode, false);
+modifiers_state[i] = 0;
+}
+}
+}
+
+void sdl2_process_key(struct sdl2_console *scon,
+ SDL_KeyboardEvent *ev)
+{
+int qcode = sdl2_scancode_to_qcode[ev->keysym.scancode];
+QemuConsole *con = scon ? scon->dcl.con : NULL;
+
+if (!qemu_console_is_graphic(con)) {
+if (ev->type == SDL_KEYDOWN) {
+switch (ev->keysym.scancode) {
+case SDL_SCANCODE_RETURN:
+kbd_put_keysym_console(con, '\n');
+break;
+case SDL_SCANCODE_BACKSPACE:
+kbd_put_keysym_console(con, QEMU_KEY_BACKSPACE);
+break;
+default:
+kbd_put_qcode_console(con, qcode);
+break;
+}
+}
+return;
+}
+
+switch (ev->keysym.scancode) {
+#if 0
+case SDL_SCANCODE_NUMLOCKCLEAR:
+case SDL_SCANCODE_CAPSLOCK:
+/* SDL does not send the key up event, so we generate it */
+qemu_input_event_send_key_qcode(con, qcode, true);
+qemu_input_event_send_key_qcode(con, qcode, false);
+return;
+#endif
+case SDL_SCANCODE_LCTRL:
+case SDL_SCANCODE_LSHIFT:
+case SDL_SCANCODE_LALT:
+case SDL_SCANCODE_LGUI:
+case SDL_SCANCODE_RCTRL:
+case SDL_SCANCODE_RSHIFT:
+case SDL_SCANCODE_RALT:
+case SDL_SCANCODE_RGUI:
+if (ev->type == SDL_KEYUP) {
+modifiers_state[ev->keysym.scancode] = 0;
+} else {
+modifiers_state[ev->keysym.scancode] = 1;
+}
+/* fall though */
+default:
+qemu_input_event_send_key_qcode(con, qcode,
+ev->type == SDL_KEYDOWN);
+}
+}
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 375e1a3..b8d592f 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -35,8 +35,6 @@
#include "ui/sdl2.h"
#include "sysemu/sysemu.h"
-#include "sdl2-keymap.h"
-
static int sdl
[Qemu-devel] [PULL 10/12] sdl2: move sdl2_2d_refresh to sdl2-2d.c
Now that common event handling code is split off, we can move
over sdl_refresh to sdl2-2d.c, and rename it to sdl2_2d_refresh.
Signed-off-by: Gerd Hoffmann
Reviewed-by: Max Reitz
---
include/ui/sdl2.h | 1 +
ui/sdl2-2d.c | 8
ui/sdl2.c | 10 +-
3 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
index bda60f8..f56c596 100644
--- a/include/ui/sdl2.h
+++ b/include/ui/sdl2.h
@@ -26,6 +26,7 @@ void sdl2_2d_update(DisplayChangeListener *dcl,
int x, int y, int w, int h);
void sdl2_2d_switch(DisplayChangeListener *dcl,
DisplaySurface *new_surface);
+void sdl2_2d_refresh(DisplayChangeListener *dcl);
void sdl2_2d_redraw(struct sdl2_console *scon);
#endif /* SDL2_H */
diff --git a/ui/sdl2-2d.c b/ui/sdl2-2d.c
index 40a552c..9264817 100644
--- a/ui/sdl2-2d.c
+++ b/ui/sdl2-2d.c
@@ -103,6 +103,14 @@ void sdl2_2d_switch(DisplayChangeListener *dcl,
sdl2_2d_redraw(scon);
}
+void sdl2_2d_refresh(DisplayChangeListener *dcl)
+{
+struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
+
+graphic_hw_update(dcl->con);
+sdl2_poll_events(scon);
+}
+
void sdl2_2d_redraw(struct sdl2_console *scon)
{
if (!scon->surface) {
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 2fdf8df..64ce206 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -594,14 +594,6 @@ void sdl2_poll_events(struct sdl2_console *scon)
}
}
-static void sdl_refresh(DisplayChangeListener *dcl)
-{
-struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
-
-graphic_hw_update(dcl->con);
-sdl2_poll_events(scon);
-}
-
static void sdl_mouse_warp(DisplayChangeListener *dcl,
int x, int y, int on)
{
@@ -667,7 +659,7 @@ static const DisplayChangeListenerOps dcl_2d_ops = {
.dpy_name = "sdl2-2d",
.dpy_gfx_update= sdl2_2d_update,
.dpy_gfx_switch= sdl2_2d_switch,
-.dpy_refresh = sdl_refresh,
+.dpy_refresh = sdl2_2d_refresh,
.dpy_mouse_set = sdl_mouse_warp,
.dpy_cursor_define = sdl_mouse_define,
};
--
1.8.3.1
[Qemu-devel] [PULL 07/12] sdl2: move sdl_switch to sdl2-2d.c
Move sdl_switch to sdl2-2d.c file, rename to sdl2_2d_switch.
Signed-off-by: Gerd Hoffmann
Reviewed-by: Max Reitz
---
include/ui/sdl2.h | 6 ++
ui/sdl2-2d.c | 42 ++
ui/sdl2.c | 50 --
3 files changed, 52 insertions(+), 46 deletions(-)
diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
index 84c27f0..fd6bfdd 100644
--- a/include/ui/sdl2.h
+++ b/include/ui/sdl2.h
@@ -13,11 +13,17 @@ struct sdl2_console {
int hidden;
};
+void sdl2_window_create(struct sdl2_console *scon);
+void sdl2_window_destroy(struct sdl2_console *scon);
+void sdl2_window_resize(struct sdl2_console *scon);
+
void sdl2_reset_keys(struct sdl2_console *scon);
void sdl2_process_key(struct sdl2_console *scon,
SDL_KeyboardEvent *ev);
void sdl2_2d_update(DisplayChangeListener *dcl,
int x, int y, int w, int h);
+void sdl2_2d_switch(DisplayChangeListener *dcl,
+DisplaySurface *new_surface);
#endif /* SDL2_H */
diff --git a/ui/sdl2-2d.c b/ui/sdl2-2d.c
index 7b0039b..29ada53 100644
--- a/ui/sdl2-2d.c
+++ b/ui/sdl2-2d.c
@@ -59,3 +59,45 @@ void sdl2_2d_update(DisplayChangeListener *dcl,
SDL_RenderCopy(scon->real_renderer, scon->texture, &rect, &rect);
SDL_RenderPresent(scon->real_renderer);
}
+
+void sdl2_2d_switch(DisplayChangeListener *dcl,
+DisplaySurface *new_surface)
+{
+struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
+DisplaySurface *old_surface = scon->surface;
+int format = 0;
+
+scon->surface = new_surface;
+
+if (scon->texture) {
+SDL_DestroyTexture(scon->texture);
+scon->texture = NULL;
+}
+
+if (!new_surface) {
+sdl2_window_destroy(scon);
+return;
+}
+
+if (!scon->real_window) {
+sdl2_window_create(scon);
+} else if (old_surface &&
+ ((surface_width(old_surface) != surface_width(new_surface)) ||
+(surface_height(old_surface) != surface_height(new_surface
{
+sdl2_window_resize(scon);
+}
+
+SDL_RenderSetLogicalSize(scon->real_renderer,
+ surface_width(new_surface),
+ surface_height(new_surface));
+
+if (surface_bits_per_pixel(scon->surface) == 16) {
+format = SDL_PIXELFORMAT_RGB565;
+} else if (surface_bits_per_pixel(scon->surface) == 32) {
+format = SDL_PIXELFORMAT_ARGB;
+}
+scon->texture = SDL_CreateTexture(scon->real_renderer, format,
+ SDL_TEXTUREACCESS_STREAMING,
+ surface_width(new_surface),
+ surface_height(new_surface));
+}
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 59b67a6..eff9cb3 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -68,7 +68,7 @@ static struct sdl2_console *get_scon_from_window(uint32_t
window_id)
return NULL;
}
-static void sdl2_window_create(struct sdl2_console *scon)
+void sdl2_window_create(struct sdl2_console *scon)
{
int flags = 0;
@@ -95,7 +95,7 @@ static void sdl2_window_create(struct sdl2_console *scon)
sdl_update_caption(scon);
}
-static void sdl2_window_destroy(struct sdl2_console *scon)
+void sdl2_window_destroy(struct sdl2_console *scon)
{
if (!scon->real_window) {
return;
@@ -107,7 +107,7 @@ static void sdl2_window_destroy(struct sdl2_console *scon)
scon->real_window = NULL;
}
-static void sdl2_window_resize(struct sdl2_console *scon)
+void sdl2_window_resize(struct sdl2_console *scon)
{
if (!scon->real_window) {
return;
@@ -118,48 +118,6 @@ static void sdl2_window_resize(struct sdl2_console *scon)
surface_height(scon->surface));
}
-static void sdl_switch(DisplayChangeListener *dcl,
- DisplaySurface *new_surface)
-{
-struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
-DisplaySurface *old_surface = scon->surface;
-int format = 0;
-
-scon->surface = new_surface;
-
-if (scon->texture) {
-SDL_DestroyTexture(scon->texture);
-scon->texture = NULL;
-}
-
-if (!new_surface) {
-sdl2_window_destroy(scon);
-return;
-}
-
-if (!scon->real_window) {
-sdl2_window_create(scon);
-} else if (old_surface &&
- ((surface_width(old_surface) != surface_width(new_surface)) ||
-(surface_height(old_surface) != surface_height(new_surface
{
-sdl2_window_resize(scon);
-}
-
-SDL_RenderSetLogicalSize(scon->real_renderer,
- surface_width(new_surface),
- surface_height(new_surface));
-
-if (surface_bits_per_pixel(scon->surface) == 16) {
-format = SDL_PIXELFORMAT_RGB565;
-} else if (surface_bits_per_pixel(scon->surface) == 32) {
-format =
[Qemu-devel] [PULL 11/12] sdl2: Use correct sdl2_console for window events
From: Max Reitz
SDL_PollEvent() polls events for all windows; therefore,
sdl2_poll_events() will poll the events for all windows and not only for
the one identified by the given sdl2_console.
This should be considered in handle_windowevent(): The window affected
by the event is not necessarily the one identified by the sdl2_console
object given to sdl2_poll_events(), but the one identified by
ev->window.windowID.
Signed-off-by: Max Reitz
Signed-off-by: Gerd Hoffmann
---
ui/sdl2.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 64ce206..04bbcc7 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -507,8 +507,10 @@ static void handle_mousewheel(SDL_Event *ev)
qemu_input_event_sync();
}
-static void handle_windowevent(struct sdl2_console *scon, SDL_Event *ev)
+static void handle_windowevent(SDL_Event *ev)
{
+struct sdl2_console *scon = get_scon_from_window(ev->window.windowID);
+
switch (ev->window.event) {
case SDL_WINDOWEVENT_RESIZED:
{
@@ -586,7 +588,7 @@ void sdl2_poll_events(struct sdl2_console *scon)
handle_mousewheel(ev);
break;
case SDL_WINDOWEVENT:
-handle_windowevent(scon, ev);
+handle_windowevent(ev);
break;
default:
break;
--
1.8.3.1
[Qemu-devel] [PULL 09/12] sdl2: factor out sdl2_poll_events
Create a new function to poll and handle sdl2 events,
which is then just called from the refresh timer.
Signed-off-by: Gerd Hoffmann
Reviewed-by: Max Reitz
---
include/ui/sdl2.h | 1 +
ui/sdl2.c | 23 +--
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
index d0ce0b0..bda60f8 100644
--- a/include/ui/sdl2.h
+++ b/include/ui/sdl2.h
@@ -16,6 +16,7 @@ struct sdl2_console {
void sdl2_window_create(struct sdl2_console *scon);
void sdl2_window_destroy(struct sdl2_console *scon);
void sdl2_window_resize(struct sdl2_console *scon);
+void sdl2_poll_events(struct sdl2_console *scon);
void sdl2_reset_keys(struct sdl2_console *scon);
void sdl2_process_key(struct sdl2_console *scon,
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 064f18a..2fdf8df 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -507,10 +507,8 @@ static void handle_mousewheel(SDL_Event *ev)
qemu_input_event_sync();
}
-static void handle_windowevent(DisplayChangeListener *dcl, SDL_Event *ev)
+static void handle_windowevent(struct sdl2_console *scon, SDL_Event *ev)
{
-struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
-
switch (ev->window.event) {
case SDL_WINDOWEVENT_RESIZED:
{
@@ -537,10 +535,10 @@ static void handle_windowevent(DisplayChangeListener
*dcl, SDL_Event *ev)
}
break;
case SDL_WINDOWEVENT_RESTORED:
-update_displaychangelistener(dcl, GUI_REFRESH_INTERVAL_DEFAULT);
+update_displaychangelistener(&scon->dcl, GUI_REFRESH_INTERVAL_DEFAULT);
break;
case SDL_WINDOWEVENT_MINIMIZED:
-update_displaychangelistener(dcl, 500);
+update_displaychangelistener(&scon->dcl, 500);
break;
case SDL_WINDOWEVENT_CLOSE:
if (!no_quit) {
@@ -551,9 +549,8 @@ static void handle_windowevent(DisplayChangeListener *dcl,
SDL_Event *ev)
}
}
-static void sdl_refresh(DisplayChangeListener *dcl)
+void sdl2_poll_events(struct sdl2_console *scon)
{
-struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
SDL_Event ev1, *ev = &ev1;
if (scon->last_vm_running != runstate_is_running()) {
@@ -561,8 +558,6 @@ static void sdl_refresh(DisplayChangeListener *dcl)
sdl_update_caption(scon);
}
-graphic_hw_update(dcl->con);
-
while (SDL_PollEvent(ev)) {
switch (ev->type) {
case SDL_KEYDOWN:
@@ -591,7 +586,7 @@ static void sdl_refresh(DisplayChangeListener *dcl)
handle_mousewheel(ev);
break;
case SDL_WINDOWEVENT:
-handle_windowevent(dcl, ev);
+handle_windowevent(scon, ev);
break;
default:
break;
@@ -599,6 +594,14 @@ static void sdl_refresh(DisplayChangeListener *dcl)
}
}
+static void sdl_refresh(DisplayChangeListener *dcl)
+{
+struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
+
+graphic_hw_update(dcl->con);
+sdl2_poll_events(scon);
+}
+
static void sdl_mouse_warp(DisplayChangeListener *dcl,
int x, int y, int on)
{
--
1.8.3.1
[Qemu-devel] [PULL 08/12] sdl2: add+use sdl2_2d_redraw function.
Add a new sdl2_2d_redraw function for a complete screen refresh,
so we can stop using graphic_hw_invalidate for that. There is
no need to bother console / gfx emulation code if we are just
going to re-blit the screen after window resizes.
Signed-off-by: Gerd Hoffmann
Reviewed-by: Max Reitz
---
include/ui/sdl2.h | 1 +
ui/sdl2-2d.c | 11 +++
ui/sdl2.c | 17 ++---
3 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
index fd6bfdd..d0ce0b0 100644
--- a/include/ui/sdl2.h
+++ b/include/ui/sdl2.h
@@ -25,5 +25,6 @@ void sdl2_2d_update(DisplayChangeListener *dcl,
int x, int y, int w, int h);
void sdl2_2d_switch(DisplayChangeListener *dcl,
DisplaySurface *new_surface);
+void sdl2_2d_redraw(struct sdl2_console *scon);
#endif /* SDL2_H */
diff --git a/ui/sdl2-2d.c b/ui/sdl2-2d.c
index 29ada53..40a552c 100644
--- a/ui/sdl2-2d.c
+++ b/ui/sdl2-2d.c
@@ -100,4 +100,15 @@ void sdl2_2d_switch(DisplayChangeListener *dcl,
SDL_TEXTUREACCESS_STREAMING,
surface_width(new_surface),
surface_height(new_surface));
+sdl2_2d_redraw(scon);
+}
+
+void sdl2_2d_redraw(struct sdl2_console *scon)
+{
+if (!scon->surface) {
+return;
+}
+sdl2_2d_update(&scon->dcl, 0, 0,
+ surface_width(scon->surface),
+ surface_height(scon->surface));
}
diff --git a/ui/sdl2.c b/ui/sdl2.c
index eff9cb3..064f18a 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -316,8 +316,7 @@ static void toggle_full_screen(struct sdl2_console *scon)
}
SDL_SetWindowFullscreen(scon->real_window, 0);
}
-graphic_hw_invalidate(scon->dcl.con);
-graphic_hw_update(scon->dcl.con);
+sdl2_2d_redraw(scon);
}
static void handle_keydown(SDL_Event *ev)
@@ -365,8 +364,8 @@ static void handle_keydown(SDL_Event *ev)
case SDL_SCANCODE_U:
sdl2_window_destroy(scon);
sdl2_window_create(scon);
-graphic_hw_invalidate(scon->dcl.con);
-graphic_hw_update(scon->dcl.con);
+/* re-create texture */
+sdl2_2d_switch(&scon->dcl, scon->surface);
gui_keysym = 1;
break;
#if 0
@@ -385,8 +384,7 @@ static void handle_keydown(SDL_Event *ev)
fprintf(stderr, "%s: scale to %dx%d\n",
__func__, width, height);
sdl_scale(scon, width, height);
-graphic_hw_invalidate(NULL);
-graphic_hw_update(NULL);
+sdl2_2d_redraw(scon);
gui_keysym = 1;
}
#endif
@@ -511,7 +509,6 @@ static void handle_mousewheel(SDL_Event *ev)
static void handle_windowevent(DisplayChangeListener *dcl, SDL_Event *ev)
{
-int w, h;
struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
switch (ev->window.event) {
@@ -523,12 +520,10 @@ static void handle_windowevent(DisplayChangeListener
*dcl, SDL_Event *ev)
info.height = ev->window.data2;
dpy_set_ui_info(scon->dcl.con, &info);
}
-graphic_hw_invalidate(scon->dcl.con);
-graphic_hw_update(scon->dcl.con);
+sdl2_2d_redraw(scon);
break;
case SDL_WINDOWEVENT_EXPOSED:
-SDL_GetWindowSize(SDL_GetWindowFromID(ev->window.windowID), &w, &h);
-sdl2_2d_update(dcl, 0, 0, w, h);
+sdl2_2d_redraw(scon);
break;
case SDL_WINDOWEVENT_FOCUS_GAINED:
case SDL_WINDOWEVENT_ENTER:
--
1.8.3.1
[Qemu-devel] [PULL 12/12] sdl2: Work around SDL2 SDL_ShowWindow() bug
From: Max Reitz
Apparently it is possible for X to send an event to a hidden SDL2
window, leading to SDL2 believing it is now shown. SDL2 will pass the
SDL_WINDOWEVENT_SHOWN message to the application without actually
showing the window; the problem is that the next SDL_ShowWindow() will
be a no-op because SDL2 assumes the window is already shown.
The correct way to react to SDL_WINDOWEVENT_SHOWN would be to clear
scon->hidden (analogous for SDL_WINDOWEVENT_HIDDEN). However, due to the
window not actually being shown, this will somehow not be correct after
all.
Therefore, just hide the window on SDL_WINDOWEVENT_SHOWN if it is
supposed to be hidden (and analogous for SDL_WINDOWEVENT_HIDDEN).
Signed-off-by: Max Reitz
Signed-off-by: Gerd Hoffmann
---
ui/sdl2.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 04bbcc7..1ae2781 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -548,6 +548,16 @@ static void handle_windowevent(SDL_Event *ev)
qemu_system_shutdown_request();
}
break;
+case SDL_WINDOWEVENT_SHOWN:
+if (scon->hidden) {
+SDL_HideWindow(scon->real_window);
+}
+break;
+case SDL_WINDOWEVENT_HIDDEN:
+if (!scon->hidden) {
+SDL_ShowWindow(scon->real_window);
+}
+break;
}
}
--
1.8.3.1
[Qemu-devel] [PULL 05/12] sdl2: move sdl_update to new sdl2-2d.c
Create new sdl2-2d file for 2d display rendering.
Move over sdl_update code, and rename to sdl2_2d_update.
Signed-off-by: Gerd Hoffmann
Reviewed-by: Max Reitz
---
include/ui/sdl2.h | 3 +++
ui/Makefile.objs | 2 +-
ui/sdl2-2d.c | 61 +++
ui/sdl2.c | 35 +--
4 files changed, 70 insertions(+), 31 deletions(-)
create mode 100644 ui/sdl2-2d.c
diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
index 77d800e..84c27f0 100644
--- a/include/ui/sdl2.h
+++ b/include/ui/sdl2.h
@@ -17,4 +17,7 @@ void sdl2_reset_keys(struct sdl2_console *scon);
void sdl2_process_key(struct sdl2_console *scon,
SDL_KeyboardEvent *ev);
+void sdl2_2d_update(DisplayChangeListener *dcl,
+int x, int y, int w, int h);
+
#endif /* SDL2_H */
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index 011c5bb..13b5cfb 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -20,7 +20,7 @@ ifeq ($(CONFIG_SDLABI),1.2)
sdl.mo-objs := sdl.o sdl_zoom.o
endif
ifeq ($(CONFIG_SDLABI),2.0)
-sdl.mo-objs := sdl2.o sdl2-input.o
+sdl.mo-objs := sdl2.o sdl2-input.o sdl2-2d.o
endif
sdl.mo-cflags := $(SDL_CFLAGS)
diff --git a/ui/sdl2-2d.c b/ui/sdl2-2d.c
new file mode 100644
index 000..7b0039b
--- /dev/null
+++ b/ui/sdl2-2d.c
@@ -0,0 +1,61 @@
+/*
+ * QEMU SDL display driver
+ *
+ * Copyright (c) 2003 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+/* Ported SDL 1.2 code to 2.0 by Dave Airlie. */
+
+/* Avoid compiler warning because macro is redefined in SDL_syswm.h. */
+#undef WIN32_LEAN_AND_MEAN
+
+#include
+#include
+
+#include "qemu-common.h"
+#include "ui/console.h"
+#include "ui/input.h"
+#include "ui/sdl2.h"
+#include "sysemu/sysemu.h"
+
+void sdl2_2d_update(DisplayChangeListener *dcl,
+int x, int y, int w, int h)
+{
+struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
+DisplaySurface *surf = qemu_console_surface(dcl->con);
+SDL_Rect rect;
+
+if (!surf) {
+return;
+}
+if (!scon->texture) {
+return;
+}
+
+rect.x = x;
+rect.y = y;
+rect.w = w;
+rect.h = h;
+
+SDL_UpdateTexture(scon->texture, NULL, surface_data(surf),
+ surface_stride(surf));
+SDL_RenderCopy(scon->real_renderer, scon->texture, &rect, &rect);
+SDL_RenderPresent(scon->real_renderer);
+}
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 9b66017..47a757a 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -72,31 +72,6 @@ static struct sdl2_console *get_scon_from_window(uint32_t
window_id)
return NULL;
}
-static void sdl_update(DisplayChangeListener *dcl,
- int x, int y, int w, int h)
-{
-struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
-SDL_Rect rect;
-DisplaySurface *surf = qemu_console_surface(dcl->con);
-
-if (!surf) {
-return;
-}
-if (!scon->texture) {
-return;
-}
-
-rect.x = x;
-rect.y = y;
-rect.w = w;
-rect.h = h;
-
-SDL_UpdateTexture(scon->texture, NULL, surface_data(surf),
- surface_stride(surf));
-SDL_RenderCopy(scon->real_renderer, scon->texture, &rect, &rect);
-SDL_RenderPresent(scon->real_renderer);
-}
-
static void do_sdl_resize(struct sdl2_console *scon, int width, int height,
int bpp)
{
@@ -609,7 +584,7 @@ static void handle_windowevent(DisplayChangeListener *dcl,
SDL_Event *ev)
break;
case SDL_WINDOWEVENT_EXPOSED:
SDL_GetWindowSize(SDL_GetWindowFromID(ev->window.windowID), &w, &h);
-sdl_update(dcl, 0, 0, w, h);
+sdl2_2d_update(dcl, 0, 0, w, h);
break;
case SDL_WINDOWEVENT_FOCUS_GAINED:
case SDL_WINDOWEVENT_ENTER:
@@ -746,9 +721,9 @@ static void sdl_cleanup(void)
SDL_QuitSubSystem(SDL_INIT_VIDEO);
}
-static const DisplayChangeLis
[Qemu-devel] [PULL 04/12] sdl2: turn on keyboard grabs
Makes quite some keys actually go to the guest instead of
being captured by the host window manager.
Signed-off-by: Gerd Hoffmann
Reviewed-by: Max Reitz
---
ui/sdl2.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/ui/sdl2.c b/ui/sdl2.c
index b8d592f..9b66017 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -189,11 +189,11 @@ static void sdl_update_caption(struct sdl2_console *scon)
status = " [Stopped]";
} else if (gui_grab) {
if (alt_grab) {
-status = " - Press Ctrl-Alt-Shift to exit mouse grab";
+status = " - Press Ctrl-Alt-Shift to exit grab";
} else if (ctrl_grab) {
-status = " - Press Right-Ctrl to exit mouse grab";
+status = " - Press Right-Ctrl to exit grab";
} else {
-status = " - Press Ctrl-Alt to exit mouse grab";
+status = " - Press Ctrl-Alt to exit grab";
}
}
@@ -785,6 +785,7 @@ void sdl_display_init(DisplayState *ds, int full_screen,
int no_frame)
SDL_GetError());
exit(1);
}
+SDL_SetHint(SDL_HINT_GRAB_KEYBOARD, "1");
for (i = 0;; i++) {
QemuConsole *con = qemu_console_lookup_by_index(i);
--
1.8.3.1
[Qemu-devel] [PULL 01/12] sdl: move version logic from source code to makefile
Compile sdl.c / sdl2.c depending on CONFIG_SDLABI instead of compiling both and have version #ifdefs in the source code. Signed-off-by: Gerd Hoffmann Reviewed-by: Max Reitz --- ui/Makefile.objs | 7 ++- ui/sdl.c | 3 --- ui/sdl2.c| 3 --- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/ui/Makefile.objs b/ui/Makefile.objs index 801cba2..b25e85f 100644 --- a/ui/Makefile.objs +++ b/ui/Makefile.objs @@ -16,7 +16,12 @@ common-obj-$(CONFIG_CURSES) += curses.o common-obj-$(CONFIG_VNC) += $(vnc-obj-y) common-obj-$(CONFIG_GTK) += gtk.o x_keymap.o -sdl.mo-objs := sdl.o sdl_zoom.o sdl2.o +ifeq ($(CONFIG_SDLABI),1.2) +sdl.mo-objs := sdl.o sdl_zoom.o +endif +ifeq ($(CONFIG_SDLABI),2.0) +sdl.mo-objs := sdl2.o +endif sdl.mo-cflags := $(SDL_CFLAGS) gtk.o-cflags := $(GTK_CFLAGS) $(VTE_CFLAGS) diff --git a/ui/sdl.c b/ui/sdl.c index 94c1d9d..3e9d810 100644 --- a/ui/sdl.c +++ b/ui/sdl.c @@ -26,8 +26,6 @@ #undef WIN32_LEAN_AND_MEAN #include - -#if SDL_MAJOR_VERSION == 1 #include #include "qemu-common.h" @@ -958,4 +956,3 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame) atexit(sdl_cleanup); } -#endif diff --git a/ui/sdl2.c b/ui/sdl2.c index 1ad74ba..45f23b1 100644 --- a/ui/sdl2.c +++ b/ui/sdl2.c @@ -27,8 +27,6 @@ #undef WIN32_LEAN_AND_MEAN #include - -#if SDL_MAJOR_VERSION == 2 #include #include "qemu-common.h" @@ -912,4 +910,3 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame) atexit(sdl_cleanup); } -#endif -- 1.8.3.1
Re: [Qemu-devel] [PATCH V2 3/8] pc: acpi-build: cleanup AcpiPmInfo initialization
On 16.12.2014 11:58, Igor Mammedov wrote:
> zero initialize AcpiPmInfo struct to reduce code bloat
> a little bit.
>
> Signed-off-by: Igor Mammedov
> ---
> hw/i386/acpi-build.c | 8 ++--
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 1fb92e5..f5ec66a 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -161,6 +161,8 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
> Object *obj = NULL;
> QObject *o;
>
> +memset(pm, 0, sizeof(*pm));
> +
> if (piix) {
> obj = piix;
> }
> @@ -173,22 +175,16 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
> o = object_property_get_qobject(obj, ACPI_PM_PROP_S3_DISABLED, NULL);
> if (o) {
> pm->s3_disabled = qint_get_int(qobject_to_qint(o));
> -} else {
> -pm->s3_disabled = false;
> }
> qobject_decref(o);
> o = object_property_get_qobject(obj, ACPI_PM_PROP_S4_DISABLED, NULL);
> if (o) {
> pm->s4_disabled = qint_get_int(qobject_to_qint(o));
> -} else {
> -pm->s4_disabled = false;
> }
> qobject_decref(o);
> o = object_property_get_qobject(obj, ACPI_PM_PROP_S4_VAL, NULL);
> if (o) {
> pm->s4_val = qint_get_int(qobject_to_qint(o));
> -} else {
> -pm->s4_val = false;
> }
> qobject_decref(o);
>
>
Reviewed-by: Claudio Fontana
--
Claudio Fontana
Server Virtualization Architect
Huawei Technologies Duesseldorf GmbH
Riesstraße 25 - 80992 München
office: +49 89 158834 4135
mobile: +49 15253060158
Re: [Qemu-devel] [PATCH 02/14] block: Add blk_new_open()
On 2014-12-18 at 16:19, Kevin Wolf wrote: Am 11.12.2014 um 14:20 hat Max Reitz geschrieben: blk_new_with_bs() creates a BlockBackend with an empty BlockDriverState attached to it. Empty BDSs are not nice, therefore add an alternative function which combines blk_new_with_bs() with bdrv_open(). Note: In contrast to bdrv_open() which takes a BlockDriver parameter, blk_new_open() does not take such a parameter. This is because bdrv_open() opens a BlockDriverState, therefore it is naturally to be able to set the BlockDriver for that BDS. The fact that bdrv_open() can open more than a single BDS is merely some form of a byproduct. blk_new_open() on the other hand is intended to be used to create a whole tree of BlockDriverStates. Therefore, setting a single BlockDriver does not make much sense. Instead, the drivers to be used for each of the nodes must be configured through the "options" QDict; including the driver of the root BDS. This is an interesting point. I generally agree with your reasoning, but if we did things right, the same would apply to filename and flags as well. But we don't, so we can't remove them now. I'm actually surprised that leaving out the driver option seem to work well enough. Well, just putting the filename into the QDict won't work thanks to the additional parsing that is done on the non-QDict filename; and most flags don't have any correspondence in the QDict. On the other hand, putting the driver name into the QDict is easy enough and will do exactly the same as specifying the BlockDriver directly, except for some different error messages. Should at least a TODO be left here for removing filename and flags? Yes, will do. Max
Re: [Qemu-devel] [PATCH V2 4/8] acpi: build_append_nameseg(): add padding if necessary
On 16.12.2014 11:58, Igor Mammedov wrote:
> According to ACPI spec NameSeg shorter than 4 characters
> must be padded up to 4 characters with "_" symbol.
> ACPI 5.0: 20.2.2 "Name Objects Encoding"
>
> Do it in build_append_nameseg() so that caller shouldn't know
> or care about it.
>
> Signed-off-by: Igor Mammedov
> ---
> v2:
> * simplify padding, suggested by: "Michael S. Tsirkin"
> ---
> hw/i386/acpi-build.c | 15 ++-
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index f5ec66a..2bf9a09 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -292,6 +292,8 @@ static inline void build_append_array(GArray *array,
> GArray *val)
> g_array_append_vals(array, val->data, val->len);
> }
>
> +#define ACPI_NAMESEG_LEN 4
> +
> static void GCC_FMT_ATTR(2, 3)
> build_append_nameseg(GArray *array, const char *format, ...)
> {
> @@ -304,8 +306,11 @@ build_append_nameseg(GArray *array, const char *format,
> ...)
> len = vsnprintf(s, sizeof s, format, args);
> va_end(args);
>
> -assert(len == 4);
> +assert(len <= ACPI_NAMESEG_LEN);
> +
> g_array_append_vals(array, s, len);
> +/* Pad up to ACPI_NAMESEG_LEN characters if necessary. */
> +g_array_append_vals(array, "", ACPI_NAMESEG_LEN - len);
> }
>
> /* 5.4 Definition Block Encoding */
> @@ -846,7 +851,7 @@ static void build_pci_bus_end(PCIBus *bus, void
> *bus_state)
>
> if (bus->parent_dev) {
> op = 0x82; /* DeviceOp */
> -build_append_nameseg(bus_table, "S%.02X_",
> +build_append_nameseg(bus_table, "S%.02X",
> bus->parent_dev->devfn);
> build_append_byte(bus_table, 0x08); /* NameOp */
> build_append_nameseg(bus_table, "_SUN");
> @@ -966,7 +971,7 @@ static void build_pci_bus_end(PCIBus *bus, void
> *bus_state)
> build_append_int(notify, 0x1U << i);
> build_append_byte(notify, 0x00); /* NullName */
> build_append_byte(notify, 0x86); /* NotifyOp */
> -build_append_nameseg(notify, "S%.02X_", PCI_DEVFN(i, 0));
> +build_append_nameseg(notify, "S%.02X", PCI_DEVFN(i, 0));
> build_append_byte(notify, 0x69); /* Arg1Op */
>
> /* Pack it up */
> @@ -1023,7 +1028,7 @@ static void build_pci_bus_end(PCIBus *bus, void
> *bus_state)
> if (bus->parent_dev) {
> build_append_byte(parent->notify_table, '^'); /*
> ParentPrefixChar */
> build_append_byte(parent->notify_table, 0x2E); /* DualNamePrefix
> */
> -build_append_nameseg(parent->notify_table, "S%.02X_",
> +build_append_nameseg(parent->notify_table, "S%.02X",
> bus->parent_dev->devfn);
> build_append_nameseg(parent->notify_table, "PCNT");
> }
> @@ -1093,7 +1098,7 @@ build_ssdt(GArray *table_data, GArray *linker,
> GArray *sb_scope = build_alloc_array();
> uint8_t op = 0x10; /* ScopeOp */
>
> -build_append_nameseg(sb_scope, "_SB_");
> +build_append_nameseg(sb_scope, "_SB");
>
> /* build Processor object for each processor */
> for (i = 0; i < acpi_cpus; i++) {
>
Reviewed-by: Claudio Fontana
--
Claudio Fontana
Server Virtualization Architect
Huawei Technologies Duesseldorf GmbH
Riesstraße 25 - 80992 München
office: +49 89 158834 4135
mobile: +49 15253060158
Re: [Qemu-devel] [PATCH V2 5/8] acpi: move generic aml building helpers into dedictated file
On 16.12.2014 11:58, Igor Mammedov wrote:
> the will be later used for composing AML primitives
> and all that could be reused later for ARM machines
> as well.
>
> Signed-off-by: Igor Mammedov
> ---
> hw/acpi/Makefile.objs| 1 +
> hw/acpi/acpi_gen_utils.c | 166
> +++
> hw/i386/acpi-build.c | 162 +-
> include/hw/acpi/acpi_gen_utils.h | 23 ++
> 4 files changed, 192 insertions(+), 160 deletions(-)
> create mode 100644 hw/acpi/acpi_gen_utils.c
> create mode 100644 include/hw/acpi/acpi_gen_utils.h
>
> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> index acd2389..4237232 100644
> --- a/hw/acpi/Makefile.objs
> +++ b/hw/acpi/Makefile.objs
> @@ -1,3 +1,4 @@
> common-obj-$(CONFIG_ACPI) += core.o piix4.o ich9.o pcihp.o cpu_hotplug.o
> common-obj-$(CONFIG_ACPI) += memory_hotplug.o
> common-obj-$(CONFIG_ACPI) += acpi_interface.o
> +common-obj-$(CONFIG_ACPI) += acpi_gen_utils.o
> diff --git a/hw/acpi/acpi_gen_utils.c b/hw/acpi/acpi_gen_utils.c
> new file mode 100644
> index 000..82d9cb7
> --- /dev/null
> +++ b/hw/acpi/acpi_gen_utils.c
> @@ -0,0 +1,166 @@
> +#include
> +#include
> +#include
> +#include
> +#include "hw/acpi/acpi_gen_utils.h"
> +
> +GArray *build_alloc_array(void)
> +{
> +return g_array_new(false, true /* clear */, 1);
> +}
> +
> +void build_free_array(GArray *array)
> +{
> +g_array_free(array, true);
> +}
indentation seems off in the two functions above.
> +
> +void build_prepend_byte(GArray *array, uint8_t val)
> +{
> +g_array_prepend_val(array, val);
> +}
> +
> +void build_append_byte(GArray *array, uint8_t val)
> +{
> +g_array_append_val(array, val);
> +}
> +
> +void build_append_array(GArray *array, GArray *val)
> +{
> +g_array_append_vals(array, val->data, val->len);
> +}
> +
> +#define ACPI_NAMESEG_LEN 4
> +
> +void GCC_FMT_ATTR(2, 3)
> +build_append_nameseg(GArray *array, const char *format, ...)
> +{
> +/* It would be nicer to use g_string_vprintf but it's only there in 2.22
> */
> +char s[] = "";
> +int len;
> +va_list args;
> +
> +va_start(args, format);
> +len = vsnprintf(s, sizeof s, format, args);
> +va_end(args);
> +
> +assert(len <= ACPI_NAMESEG_LEN);
> +
> +g_array_append_vals(array, s, len);
> +/* Pad up to ACPI_NAMESEG_LEN characters if necessary. */
> +g_array_append_vals(array, "", ACPI_NAMESEG_LEN - len);
> +}
> +
> +/* 5.4 Definition Block Encoding */
> +enum {
> +PACKAGE_LENGTH_1BYTE_SHIFT = 6, /* Up to 63 - use extra 2 bits. */
> +PACKAGE_LENGTH_2BYTE_SHIFT = 4,
> +PACKAGE_LENGTH_3BYTE_SHIFT = 12,
> +PACKAGE_LENGTH_4BYTE_SHIFT = 20,
> +};
> +
> +void build_prepend_package_length(GArray *package, unsigned min_bytes)
> +{
> +uint8_t byte;
> +unsigned length = package->len;
> +unsigned length_bytes;
> +
> +if (length + 1 < (1 << PACKAGE_LENGTH_1BYTE_SHIFT)) {
> +length_bytes = 1;
> +} else if (length + 2 < (1 << PACKAGE_LENGTH_3BYTE_SHIFT)) {
> +length_bytes = 2;
> +} else if (length + 3 < (1 << PACKAGE_LENGTH_4BYTE_SHIFT)) {
> +length_bytes = 3;
> +} else {
> +length_bytes = 4;
> +}
> +
> +/* Force length to at least min_bytes.
> + * This wastes memory but that's how bios did it.
> + */
> +length_bytes = MAX(length_bytes, min_bytes);
> +
> +/* PkgLength is the length of the inclusive length of the data. */
> +length += length_bytes;
> +
> +switch (length_bytes) {
> +case 1:
> +byte = length;
> +build_prepend_byte(package, byte);
> +return;
> +case 4:
> +byte = length >> PACKAGE_LENGTH_4BYTE_SHIFT;
> +build_prepend_byte(package, byte);
> +length &= (1 << PACKAGE_LENGTH_4BYTE_SHIFT) - 1;
> +/* fall through */
> +case 3:
> +byte = length >> PACKAGE_LENGTH_3BYTE_SHIFT;
> +build_prepend_byte(package, byte);
> +length &= (1 << PACKAGE_LENGTH_3BYTE_SHIFT) - 1;
> +/* fall through */
> +case 2:
> +byte = length >> PACKAGE_LENGTH_2BYTE_SHIFT;
> +build_prepend_byte(package, byte);
> +length &= (1 << PACKAGE_LENGTH_2BYTE_SHIFT) - 1;
> +/* fall through */
> +}
> +/*
> + * Most significant two bits of byte zero indicate how many following
> bytes
> + * are in PkgLength encoding.
> + */
> +byte = ((length_bytes - 1) << PACKAGE_LENGTH_1BYTE_SHIFT) | length;
> +build_prepend_byte(package, byte);
> +}
> +
> +void build_package(GArray *package, uint8_t op, unsigned min_bytes)
> +{
> +build_prepend_package_length(package, min_bytes);
> +build_prepend_byte(package, op);
> +}
> +
> +void build_extop_package(GArray *package, uint8_t op)
> +{
> +build_package(package, op, 1);
> +build_prepend_byte(package, 0x5B); /* ExtOpPrefix */
> +}
> +
> +void build_append_value(GArray *table, uint32_t value, i
Re: [Qemu-devel] [PATCH v5 00/11] fw_cfg, bootorder, and UEFI+'-kernel' on arm/virt
Hi,
Attached is a patch drafting a dma interface for fw_cfg, lousily based
on the ideas I've outlined a few days ago on this list. Applies on top
of this patch series.
Host side only, unfinished, untested. Mainly sending this out as
notification that I've looked into this, to avoid duplicating work.
I'll go disappear into xmas vacation in a few hours and will not
continue with this until next year.
My plan is to coordinate in January with Laszlo how to go forward. If
someone feels bored during the holidays feel free to grab it and run
with it.
cheers,
Gerd
>From 679b076610ada4229a735caf01dd1390ef336788 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann
Date: Thu, 18 Dec 2014 17:03:52 +0100
Subject: [PATCH] [wip] fw_cfg dma interface
First draft of a fw_cfg dma interface. Designed as add-on to the
extisting fw_cfg interface, i.e. there is no select register. There
are four 32bit registers: Target address (low and high bits), transfer
length, control register.
Protocol:
Probing for dma support being available: Write target address, read it
back, verify you got back the value you wrote.
Kick a transfer: Write select, target address and transfer length
registers (order doesn't matter). Then flip read or write bit in the
control register to '1'. Also make sure the error bit is cleared.
Check result: Read control register:
error bit set -> something went wrong.
all bits cleared -> transfer finished successfully.
otherwise -> transfer still in progress (doesn't happen
today due to implementation not being async,
but may in the future).
Target address goes up and transfer length goes down as the transfer
happens, so after a successfull transfer the length register is zero
and the address register points right after the memory block written.
If a partial transfer happened before an error occured the address and
length registers indicate how much data has been transfered
successfully.
Todo list:
* figure which address space to use.
* wind up in boards.
* write guest support.
* test the whole thing.
Possible improvements (can be done incremental):
* Better error reporting. Right now we throw errors on invalid
addresses only. We could also throw errors on invalid reads/writes
instead of using fw_cfg_read (return zeros on error) and fw_cfg_write
(silently discard data on error) behavior.
* Use memcpy instead of calling fw_cfg_read in a loop.
Signed-off-by: Gerd Hoffmann
---
hw/nvram/fw_cfg.c | 167 +-
1 file changed, 165 insertions(+), 2 deletions(-)
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 33ffd0f..75ce403 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -23,6 +23,7 @@
*/
#include "hw/hw.h"
#include "sysemu/sysemu.h"
+#include "sysemu/dma.h"
#include "hw/isa/isa.h"
#include "hw/nvram/fw_cfg.h"
#include "hw/sysbus.h"
@@ -42,6 +43,19 @@
#define FW_CFG_IO(obj) OBJECT_CHECK(FWCfgIoState, (obj), TYPE_FW_CFG_IO)
#define FW_CFG_MEM(obj) OBJECT_CHECK(FWCfgMemState, (obj), TYPE_FW_CFG_MEM)
+/* fw_cfg dma registers */
+#define FW_CFG_DMA_ADDR_LO0
+#define FW_CFG_DMA_ADDR_HI4
+#define FW_CFG_DMA_LENGTH 8
+#define FW_CFG_DMA_CONTROL 12
+#define FW_CFG_DMA_SIZE 16
+
+/* FW_CFG_DMA_CONTROL bits */
+#define FW_CFG_DMA_CTL_READ0x01
+#define FW_CFG_DMA_CTL_WRITE 0x02
+#define FW_CFG_DMA_CTL_ERROR 0x04
+#define FW_CFG_DMA_CTL_MASK0x07
+
typedef struct FWCfgEntry {
uint32_t len;
uint8_t *data;
@@ -60,6 +74,12 @@ struct FWCfgState {
uint16_t cur_entry;
uint32_t cur_offset;
Notifier machine_ready;
+
+bool dma_enabled;
+AddressSpace *dma_as;
+dma_addr_t dma_addr;
+uint32_t dma_len;
+uint32_t dma_ctl;
};
struct FWCfgIoState {
@@ -76,8 +96,8 @@ struct FWCfgMemState {
FWCfgState parent_obj;
/*< public >*/
-MemoryRegion ctl_iomem, data_iomem;
-hwaddr ctl_addr, data_addr;
+MemoryRegion ctl_iomem, data_iomem, dma_iomem;
+hwaddr ctl_addr, data_addr, dma_addr;
uint32_t data_width;
MemoryRegionOps wide_data_ops;
};
@@ -335,6 +355,102 @@ static void fw_cfg_data_mem_write(void *opaque, hwaddr addr,
}
}
+static void fw_cfg_dma_transfer(FWCfgState *s)
+{
+DMADirection direction;
+dma_addr_t len;
+uint8_t *ptr;
+uint32_t i;
+
+if ((s->dma_ctl & FW_CFG_DMA_CTL_READ) &&
+(s->dma_ctl & FW_CFG_DMA_CTL_WRITE)) {
+s->dma_ctl |= FW_CFG_DMA_CTL_ERROR;
+return;
+}
+
+if (s->dma_ctl & FW_CFG_DMA_CTL_ERROR) {
+return;
+} else if (s->dma_ctl & FW_CFG_DMA_CTL_READ) {
+direction = DMA_DIRECTION_FROM_DEVICE;
+} else if (s->dma_ctl & FW_CFG_DMA_CTL_WRITE) {
+direction = DMA_DIRECTION_TO_DEVICE;
+} else {
+return;
+}
+
+while (s->dma_len > 0) {
+len = s->dma_len;
+ptr = dma_memory_map(s->d
Re: [Qemu-devel] [PATCH V2 5/8] acpi: move generic aml building helpers into dedictated file
On Fri, 19 Dec 2014 10:31:18 +0100
Claudio Fontana wrote:
> On 16.12.2014 11:58, Igor Mammedov wrote:
> > the will be later used for composing AML primitives
> > and all that could be reused later for ARM machines
> > as well.
> >
> > Signed-off-by: Igor Mammedov
> > ---
> > hw/acpi/Makefile.objs| 1 +
> > hw/acpi/acpi_gen_utils.c | 166
> > +++
> > hw/i386/acpi-build.c | 162
> > +-
> > include/hw/acpi/acpi_gen_utils.h | 23 ++
> > 4 files changed, 192 insertions(+), 160 deletions(-)
> > create mode 100644 hw/acpi/acpi_gen_utils.c
> > create mode 100644 include/hw/acpi/acpi_gen_utils.h
> >
> > diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> > index acd2389..4237232 100644
> > --- a/hw/acpi/Makefile.objs
> > +++ b/hw/acpi/Makefile.objs
> > @@ -1,3 +1,4 @@
> > common-obj-$(CONFIG_ACPI) += core.o piix4.o ich9.o pcihp.o cpu_hotplug.o
> > common-obj-$(CONFIG_ACPI) += memory_hotplug.o
> > common-obj-$(CONFIG_ACPI) += acpi_interface.o
> > +common-obj-$(CONFIG_ACPI) += acpi_gen_utils.o
> > diff --git a/hw/acpi/acpi_gen_utils.c b/hw/acpi/acpi_gen_utils.c
> > new file mode 100644
> > index 000..82d9cb7
> > --- /dev/null
> > +++ b/hw/acpi/acpi_gen_utils.c
> > @@ -0,0 +1,166 @@
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include "hw/acpi/acpi_gen_utils.h"
> > +
> > +GArray *build_alloc_array(void)
> > +{
> > +return g_array_new(false, true /* clear */, 1);
> > +}
> > +
> > +void build_free_array(GArray *array)
> > +{
> > +g_array_free(array, true);
> > +}
>
> indentation seems off in the two functions above.
Moving bugs from original, I'll fix it up.
Thanks!
>
> > +
> > +void build_prepend_byte(GArray *array, uint8_t val)
> > +{
> > +g_array_prepend_val(array, val);
> > +}
> > +
> > +void build_append_byte(GArray *array, uint8_t val)
> > +{
> > +g_array_append_val(array, val);
> > +}
> > +
> > +void build_append_array(GArray *array, GArray *val)
> > +{
> > +g_array_append_vals(array, val->data, val->len);
> > +}
> > +
> > +#define ACPI_NAMESEG_LEN 4
> > +
> > +void GCC_FMT_ATTR(2, 3)
> > +build_append_nameseg(GArray *array, const char *format, ...)
> > +{
> > +/* It would be nicer to use g_string_vprintf but it's only there in
> > 2.22 */
> > +char s[] = "";
> > +int len;
> > +va_list args;
> > +
> > +va_start(args, format);
> > +len = vsnprintf(s, sizeof s, format, args);
> > +va_end(args);
> > +
> > +assert(len <= ACPI_NAMESEG_LEN);
> > +
> > +g_array_append_vals(array, s, len);
> > +/* Pad up to ACPI_NAMESEG_LEN characters if necessary. */
> > +g_array_append_vals(array, "", ACPI_NAMESEG_LEN - len);
> > +}
> > +
> > +/* 5.4 Definition Block Encoding */
> > +enum {
> > +PACKAGE_LENGTH_1BYTE_SHIFT = 6, /* Up to 63 - use extra 2 bits. */
> > +PACKAGE_LENGTH_2BYTE_SHIFT = 4,
> > +PACKAGE_LENGTH_3BYTE_SHIFT = 12,
> > +PACKAGE_LENGTH_4BYTE_SHIFT = 20,
> > +};
> > +
> > +void build_prepend_package_length(GArray *package, unsigned min_bytes)
> > +{
> > +uint8_t byte;
> > +unsigned length = package->len;
> > +unsigned length_bytes;
> > +
> > +if (length + 1 < (1 << PACKAGE_LENGTH_1BYTE_SHIFT)) {
> > +length_bytes = 1;
> > +} else if (length + 2 < (1 << PACKAGE_LENGTH_3BYTE_SHIFT)) {
> > +length_bytes = 2;
> > +} else if (length + 3 < (1 << PACKAGE_LENGTH_4BYTE_SHIFT)) {
> > +length_bytes = 3;
> > +} else {
> > +length_bytes = 4;
> > +}
> > +
> > +/* Force length to at least min_bytes.
> > + * This wastes memory but that's how bios did it.
> > + */
> > +length_bytes = MAX(length_bytes, min_bytes);
> > +
> > +/* PkgLength is the length of the inclusive length of the data. */
> > +length += length_bytes;
> > +
> > +switch (length_bytes) {
> > +case 1:
> > +byte = length;
> > +build_prepend_byte(package, byte);
> > +return;
> > +case 4:
> > +byte = length >> PACKAGE_LENGTH_4BYTE_SHIFT;
> > +build_prepend_byte(package, byte);
> > +length &= (1 << PACKAGE_LENGTH_4BYTE_SHIFT) - 1;
> > +/* fall through */
> > +case 3:
> > +byte = length >> PACKAGE_LENGTH_3BYTE_SHIFT;
> > +build_prepend_byte(package, byte);
> > +length &= (1 << PACKAGE_LENGTH_3BYTE_SHIFT) - 1;
> > +/* fall through */
> > +case 2:
> > +byte = length >> PACKAGE_LENGTH_2BYTE_SHIFT;
> > +build_prepend_byte(package, byte);
> > +length &= (1 << PACKAGE_LENGTH_2BYTE_SHIFT) - 1;
> > +/* fall through */
> > +}
> > +/*
> > + * Most significant two bits of byte zero indicate how many following
> > bytes
> > + * are in PkgLength encoding.
> > + */
> > +byte = ((length_bytes - 1) << PACKAGE_LENGTH_1BYTE_SHIFT) | length;
> > +build_prepend_byte(package, byte);
> > +}
>
Re: [Qemu-devel] [PATCH 0/2] tcg: Move TCG arch-specific initialization inside TCG code
Am 19.12.2014 um 05:26 schrieb Eduardo Habkost: > Many architectures manually call arch-specific TCG initialization at CPU init > time[1], instead of having tcg_init() doing all initialization steps. This > series introduces a tcg_arch_init() function that may be implemented by > architecture-specific code for TCG initialization. How do you imagine this to work with multiple CPU types? From the looks of it, this is for the target CPU, not the TCG architecture, so tcg_arch_init() limits us to one implementation unlike now. On the other hand it doesn't seem to be a per-CPU, i.e. CPUClass, initialization either... Regards, Andreas -- SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 21284 AG Nürnberg
Re: [Qemu-devel] [PATCH v6 0/4] qmp: Add "blockdev-backup"
On 2014-12-18 at 11:37, Fam Zheng wrote: v6: Add Eric's rev-by in 1/4. Address minor comments in 2/4, 3/4. Add John's rev-by in 3/4. v5: Address Max's and Markus' comments: Split patch 1. (Markus) Fix typos and pastos. (Markus, Max) Actually acquire aio context. (Max) Drop unnecessary initialization of fields in blockdev_backup_prepare. (Max) Add "sync" in the document example. Add Max's rev-by in patch 4. The existing drive-backup command accepts a target file path, but that interface provides little flexibility on the properties of target block device, compared to what is possible with "blockdev-add", "drive_add" or "-drive". This is also a building block to allow image fleecing (creating a point in time snapshot and export with nbd-server-add). (For symmetry, blockdev-mirror will be added in a separate series.) Fam Fam Zheng (4): qapi: Comment version info in TransactionAction qmp: Add command 'blockdev-backup' block: Add blockdev-backup to transaction qemu-iotests: Test blockdev-backup in 055 block/backup.c | 28 ++ blockdev.c | 133 qapi-schema.json | 8 ++ qapi/block-core.json | 54 qmp-commands.hx| 42 + tests/qemu-iotests/055 | 211 + tests/qemu-iotests/055.out | 4 +- 7 files changed, 441 insertions(+), 39 deletions(-) (Trusting Markus to fix QERR_* and non-ERROR_CLASS_GENERIC_ERROR uses in a follow-up) Thanks, applied to my block tree: https://github.com/XanClic/qemu/commits/block
Re: [Qemu-devel] [PATCH v5 00/11] fw_cfg, bootorder, and UEFI+'-kernel' on arm/virt
On 19.12.14 10:39, Gerd Hoffmann wrote: > Hi, > > Attached is a patch drafting a dma interface for fw_cfg, lousily based > on the ideas I've outlined a few days ago on this list. Applies on top > of this patch series. > > Host side only, unfinished, untested. Mainly sending this out as > notification that I've looked into this, to avoid duplicating work. > I'll go disappear into xmas vacation in a few hours and will not > continue with this until next year. > > My plan is to coordinate in January with Laszlo how to go forward. If > someone feels bored during the holidays feel free to grab it and run > with it. Nice first draft :). The major thing I would do differently is the way to access the DMA registers. Instead of making them even more PIO/MMIO ports, I'd rather just make them one big 256 bit fw_cfg entry. That way we don't need to modify (and synchronize) any machine port layout to support DMA. Alex
Re: [Qemu-devel] [PATCH V2 6/8] acpi: add build_append_namestring() helper
On 16.12.2014 11:58, Igor Mammedov wrote:
> Use build_append_namestring() instead of build_append_nameseg()
> So user won't have to care whether name is NameSeg, NamePath or
> NameString.
>
> See for refernce ACPI 5.0: 20.2.2 Name Objects Encoding
>
> Signed-off-by: Igor Mammedov
> ---
> hw/acpi/acpi_gen_utils.c | 86
> +++-
> hw/i386/acpi-build.c | 35
> include/hw/acpi/acpi_gen_utils.h | 2 +-
> 3 files changed, 102 insertions(+), 21 deletions(-)
>
> diff --git a/hw/acpi/acpi_gen_utils.c b/hw/acpi/acpi_gen_utils.c
> index 82d9cb7..2e52690 100644
> --- a/hw/acpi/acpi_gen_utils.c
> +++ b/hw/acpi/acpi_gen_utils.c
> @@ -31,7 +31,7 @@ void build_append_array(GArray *array, GArray *val)
>
> #define ACPI_NAMESEG_LEN 4
>
> -void GCC_FMT_ATTR(2, 3)
> +static void GCC_FMT_ATTR(2, 3)
> build_append_nameseg(GArray *array, const char *format, ...)
> {
> /* It would be nicer to use g_string_vprintf but it's only there in 2.22
> */
> @@ -50,6 +50,90 @@ build_append_nameseg(GArray *array, const char *format,
> ...)
> g_array_append_vals(array, "", ACPI_NAMESEG_LEN - len);
> }
>
> +static const uint8_t ACPI_DualNamePrefix = 0x2E;
> +static const uint8_t ACPI_MultiNamePrefix = 0x2F;
> +
> +static void
> +build_append_namestringv(GArray *array, const char *format, va_list ap)
> +{
> +/* It would be nicer to use g_string_vprintf but it's only there in 2.22
> */
> +char *s;
> +int len;
> +va_list va_len;
> +char **segs;
> +char **segs_iter;
> +int seg_count = 0;
> +
> +va_copy(va_len, ap);
> +len = vsnprintf(NULL, 0, format, va_len);
> +va_end(va_len);
> +len += 1;
> +s = g_new(typeof(*s), len);
> +
> +len = vsnprintf(s, len, format, ap);
> +
> +segs = g_strsplit(s, ".", 0);
> +g_free(s);
> +
> +/* count segments */
> +segs_iter = segs;
> +while (*segs_iter) {
> +++segs_iter;
> +++seg_count;
> +}
since SegCount is explicitly mentioned in 20.2.2 as being between 1 and 255,
why don't we assert right here (instead of below), and add an assert for
seg_count != 0 as well while we're at it?
Thanks,
Claudio
> +
> +/* handle RootPath || PrefixPath */
> +s = *segs;
> +while (*s == '\\' || *s == '^') {
> +g_array_append_val(array, *s);
> +++s;
> +}
> +
> +switch (seg_count) {
> +case 1:
> +if (!*s) { /* NullName */
> +const uint8_t nullpath = 0;
> +g_array_append_val(array, nullpath);
> +} else {
> +build_append_nameseg(array, "%s", s);
> +}
> +break;
> +
> +case 2:
> +g_array_append_val(array, ACPI_DualNamePrefix);
> +build_append_nameseg(array, "%s", s);
> +build_append_nameseg(array, "%s", segs[1]);
> +break;
> +
> +default:
> +g_array_append_val(array, ACPI_MultiNamePrefix);
> +assert(seg_count <= 0xFF); /* ByteData Max */
> +g_array_append_val(array, seg_count);
> +
> +/* handle the 1st segment manually due to prefix/root path */
> +build_append_nameseg(array, "%s", s);
> +
> +/* add the rest of segments */
> +segs_iter = segs + 1;
> +while (*segs_iter) {
> +build_append_nameseg(array, "%s", *segs_iter);
> +++segs_iter;
> +}
> +break;
> +}
> +g_strfreev(segs);
> +}
> +
> +void GCC_FMT_ATTR(2, 3)
> +build_append_namestring(GArray *array, const char *format, ...)
> +{
> +va_list ap;
> +
> +va_start(ap, format);
> +build_append_namestringv(array, format, ap);
> +va_end(ap);
> +}
> +
> /* 5.4 Definition Block Encoding */
> enum {
> PACKAGE_LENGTH_1BYTE_SHIFT = 6, /* Up to 63 - use extra 2 bits. */
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 870e4a0..0f6202d 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -273,7 +273,7 @@ static GArray *build_alloc_method(const char *name,
> uint8_t arg_count)
> {
> GArray *method = build_alloc_array();
>
> -build_append_nameseg(method, "%s", name);
> +build_append_namestring(method, "%s", name);
> build_append_byte(method, arg_count); /* MethodFlags: ArgCount */
>
> return method;
> @@ -565,7 +565,7 @@ build_append_notify_method(GArray *device, const char
> *name,
>
> for (i = 0; i < count; i++) {
> GArray *target = build_alloc_array();
> -build_append_nameseg(target, format, i);
> +build_append_namestring(target, format, i);
> assert(i < 256); /* Fits in 1 byte */
> build_append_notify_target_ifequal(method, target, i, 1);
> build_free_array(target);
> @@ -693,24 +693,24 @@ static void build_pci_bus_end(PCIBus *bus, void
> *bus_state)
>
> if (bus->parent_dev) {
> op = 0x82; /* DeviceOp */
> -build_append_nameseg(bus_table, "S%.02X",
> +build_append_namestri
Re: [Qemu-devel] [PATCH 1/5] pseries: Move sPAPR RTC code into its own file
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 19.12.14 06:39, David Gibson wrote: > On Fri, Dec 19, 2014 at 12:36:11AM +0100, Alexander Graf wrote: >> -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 >> >> >> >> On 18.12.14 06:43, David Gibson wrote: >>> On Tue, Dec 16, 2014 at 10:41:16AM +0100, Alexander Graf >>> wrote: > Am 16.12.2014 um 02:24 schrieb David Gibson > : > >> On Tue, Dec 16, 2014 at 01:59:01AM +0100, Alexander Graf >> wrote: >> >> >>> On 16.12.14 01:43, David Gibson wrote: At the moment >>> the RTAS (firmware/hypervisor) time of day functions >>> are implemented in spapr_rtas.c along with a bunch of >>> other things. Since we're going to be expanding these >>> a bit, move the RTAS RTC related code out into new >>> file spapr_rtc.c. Also add its own initialization >>> function, spapr_rtc_init() called from the main machine >>> init routine. >>> >>> Signed-off-by: David Gibson >>> --- hw/ppc/Makefile.objs >>> | 2 +- hw/ppc/spapr.c | 3 ++ >>> hw/ppc/spapr_rtas.c| 49 >>> - hw/ppc/spapr_rtc.c | >>> 83 ++ >>> include/hw/ppc/spapr.h | 1 + 5 files changed, 88 >>> insertions(+), 50 deletions(-) create mode 100644 >>> hw/ppc/spapr_rtc.c >>> >>> diff --git a/hw/ppc/Makefile.objs >>> b/hw/ppc/Makefile.objs index 19d9920..437955d 100644 >>> --- a/hw/ppc/Makefile.objs +++ b/hw/ppc/Makefile.objs >>> @@ -3,7 +3,7 @@ obj-y += ppc.o ppc_booke.o # IBM >>> pSeries (sPAPR) obj-$(CONFIG_PSERIES) += spapr.o >>> spapr_vio.o spapr_events.o obj-$(CONFIG_PSERIES) += >>> spapr_hcall.o spapr_iommu.o spapr_rtas.o >>> -obj-$(CONFIG_PSERIES) += spapr_pci.o >>> +obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o ifeq >>> ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy) >>> obj-y += spapr_pci_vfio.o endif diff --git >>> a/hw/ppc/spapr.c b/hw/ppc/spapr.c index >>> 30de25d..16377a3 100644 --- a/hw/ppc/spapr.c +++ >>> b/hw/ppc/spapr.c @@ -1446,6 +1446,9 @@ static void >>> ppc_spapr_init(MachineState *machine) /* Set up EPOW >>> events infrastructure */ spapr_events_init(spapr); >>> >>> +/* Set up the RTC RTAS interfaces */ + >>> spapr_rtc_init(); >> >> Do you think we could make it a device instead? > > Um.. I guess. Is there a standard place to put such > pseudo-devices in the bus heirarchy? How about we combine this with some cleanup work and create a new spapr bus that (in the long term) exposes all the details we carry around in the spapr struct to its children via the bus? >>> >>> I've thought about this a bit more. It really doesn't make >>> sense to put the rtc device anywhere except the root bus >>> (which optionally could become an spapr sub-type of the default >>> root bus type). >> >> Well, we could always just leave sysbus completely unused and >> instead create our own global spapr root bus. In fact, that's >> probably what we should do ;). > > We could, but I absolutely disagree that it's what we should do. > > What is the sysbus for, if not to represent devices on the system > that don't fall under some other bus bridge. Putting an spapr bus > under it is indirection for no purpose. Sysbus is a big bucket used to represent a number of buses that have devices that * are MMIO or PIO mapped into CPU visible memory regions * have hard IRQ wirings Most of the devices on sPAPR are not mapped into MMIO space at all - they have their own hcall based access methods. For IRQs, sPAPR has its own allocation algorithm as well, which is vastly different from anything we have with SysBus today. I don't think we can reuse much more of SysBusDevice other than everything that we already have in DeviceState - and at that point we can just inherit sPAPRDeviceState from DeviceState and completely ignore SysBus. > >> If that blows your mind for this patch set, we can make the RTC >> a sysbus device too though. >> >>> But, while splitting this into its own device would be cleaner, >>> I don't think we should delay real behavioural fixes for that >>> cleanup. Working out how to split out the device without >>> breaking old machine types, and keeping migration working is >>> making my head hurt. >> >> I don't think it's that hard. Just create a compat version for >> version 2 of "spapr" vmstates and have a post_load hook in there >> that shoves the rtc offset into the new rtc device via a qom >> property. You should even be able to just find the RTC device by >> path. > > Yeah, I kind of figured that out myself in the meantime. It's bit > ugly, because we need to retain the otherwise useless rtc_offset > field in sPAPREnvironment just to be loaded by the > vmstatdescription then pushed into the rtc, but I guess it works. Yeah, but i think we can
Re: [Qemu-devel] [PATCHv3 0/2] Fix virtio-serial migration on bi-endian targets
On 19.12.14 04:57, David Gibson wrote: > On a bi-endian target, with a guest in the non-default endian mode, > attempting to migrate twice in a row with a virtio-serial device wil > cause a qemu SEGV on the second outgoing migration. > > The problem is that virtio_serial_save_device() (and other places) expect > VirtIOSerial->config to be in current guest endianness. On a fresh boot, > virtio_serial_device_realize() will initialize VirtIOSerial->config in > default endianness. It's assumed the guest OS will make its true > endianness known before the device is reset and initialized, then > vser_reset adjusts VirtIOSerial->config into the new endianness. > > But on an incoming migration, the device isn't reset (after all the guest > has a running driver as far as it's concerned), which means that > VirtIOSerial->config retains its default endianness value from > virtio_serial_device_realize(). > > On a subsequent outgoing migration, virtio_serial_save_device() attempts > to interpret VirtIOSerial->config.max_nr_ports in current endianness when > its actually in default endianness and then runs off the end of the > ports_map array in the loop immediately afterwards. > > We could fix this by adjusting VirtIOSerial->config into the correct > current endianness after an incoming migration. But in fact we > already have a host endian copy of the max number of ports readily > available, so it's simpler to just use that instead. Patch 1/2 does > that. > > Once that's done, it becomes clear that there's really no reason to > keep the guest-endian copy of the config space around persistently > (config accesses aren't a fast path, so it can be constructed when > necessary). Patch 2/2 makes that cleanup. Please provide a version history of changes between versions. Also for patches you'd expect PPC people to read, CC qemu-ppc (maybe doesn't apply to this patch, but does to others) ;) Alex
Re: [Qemu-devel] [PATCH V2 6/8] acpi: add build_append_namestring() helper
On Fri, 19 Dec 2014 10:53:03 +0100
Claudio Fontana wrote:
> On 16.12.2014 11:58, Igor Mammedov wrote:
> > Use build_append_namestring() instead of build_append_nameseg()
> > So user won't have to care whether name is NameSeg, NamePath or
> > NameString.
> >
> > See for refernce ACPI 5.0: 20.2.2 Name Objects Encoding
> >
> > Signed-off-by: Igor Mammedov
> > ---
> > hw/acpi/acpi_gen_utils.c | 86
> > +++-
> > hw/i386/acpi-build.c | 35
> > include/hw/acpi/acpi_gen_utils.h | 2 +-
> > 3 files changed, 102 insertions(+), 21 deletions(-)
> >
> > diff --git a/hw/acpi/acpi_gen_utils.c b/hw/acpi/acpi_gen_utils.c
> > index 82d9cb7..2e52690 100644
> > --- a/hw/acpi/acpi_gen_utils.c
> > +++ b/hw/acpi/acpi_gen_utils.c
> > @@ -31,7 +31,7 @@ void build_append_array(GArray *array, GArray *val)
> >
> > #define ACPI_NAMESEG_LEN 4
> >
> > -void GCC_FMT_ATTR(2, 3)
> > +static void GCC_FMT_ATTR(2, 3)
> > build_append_nameseg(GArray *array, const char *format, ...)
> > {
> > /* It would be nicer to use g_string_vprintf but it's only there in
> > 2.22 */
> > @@ -50,6 +50,90 @@ build_append_nameseg(GArray *array, const char *format,
> > ...)
> > g_array_append_vals(array, "", ACPI_NAMESEG_LEN - len);
> > }
> >
> > +static const uint8_t ACPI_DualNamePrefix = 0x2E;
> > +static const uint8_t ACPI_MultiNamePrefix = 0x2F;
> > +
> > +static void
> > +build_append_namestringv(GArray *array, const char *format, va_list ap)
> > +{
> > +/* It would be nicer to use g_string_vprintf but it's only there in
> > 2.22 */
> > +char *s;
> > +int len;
> > +va_list va_len;
> > +char **segs;
> > +char **segs_iter;
> > +int seg_count = 0;
> > +
> > +va_copy(va_len, ap);
> > +len = vsnprintf(NULL, 0, format, va_len);
> > +va_end(va_len);
> > +len += 1;
> > +s = g_new(typeof(*s), len);
> > +
> > +len = vsnprintf(s, len, format, ap);
> > +
> > +segs = g_strsplit(s, ".", 0);
> > +g_free(s);
> > +
> > +/* count segments */
> > +segs_iter = segs;
> > +while (*segs_iter) {
> > +++segs_iter;
> > +++seg_count;
> > +}
>
> since SegCount is explicitly mentioned in 20.2.2 as being between 1 and 255,
> why don't we assert right here (instead of below), and add an assert for
> seg_count != 0 as well while we're at it?
Sure, I'll move it here and add extra assert as well.
>
> Thanks,
>
> Claudio
>
> > +
> > +/* handle RootPath || PrefixPath */
> > +s = *segs;
> > +while (*s == '\\' || *s == '^') {
> > +g_array_append_val(array, *s);
> > +++s;
> > +}
> > +
> > +switch (seg_count) {
> > +case 1:
> > +if (!*s) { /* NullName */
> > +const uint8_t nullpath = 0;
> > +g_array_append_val(array, nullpath);
> > +} else {
> > +build_append_nameseg(array, "%s", s);
> > +}
> > +break;
> > +
> > +case 2:
> > +g_array_append_val(array, ACPI_DualNamePrefix);
> > +build_append_nameseg(array, "%s", s);
> > +build_append_nameseg(array, "%s", segs[1]);
> > +break;
> > +
> > +default:
> > +g_array_append_val(array, ACPI_MultiNamePrefix);
> > +assert(seg_count <= 0xFF); /* ByteData Max */
> > +g_array_append_val(array, seg_count);
> > +
> > +/* handle the 1st segment manually due to prefix/root path */
> > +build_append_nameseg(array, "%s", s);
> > +
> > +/* add the rest of segments */
> > +segs_iter = segs + 1;
> > +while (*segs_iter) {
> > +build_append_nameseg(array, "%s", *segs_iter);
> > +++segs_iter;
> > +}
> > +break;
> > +}
> > +g_strfreev(segs);
> > +}
> > +
> > +void GCC_FMT_ATTR(2, 3)
> > +build_append_namestring(GArray *array, const char *format, ...)
> > +{
> > +va_list ap;
> > +
> > +va_start(ap, format);
> > +build_append_namestringv(array, format, ap);
> > +va_end(ap);
> > +}
> > +
> > /* 5.4 Definition Block Encoding */
> > enum {
> > PACKAGE_LENGTH_1BYTE_SHIFT = 6, /* Up to 63 - use extra 2 bits. */
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 870e4a0..0f6202d 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -273,7 +273,7 @@ static GArray *build_alloc_method(const char *name,
> > uint8_t arg_count)
> > {
> > GArray *method = build_alloc_array();
> >
> > -build_append_nameseg(method, "%s", name);
> > +build_append_namestring(method, "%s", name);
> > build_append_byte(method, arg_count); /* MethodFlags: ArgCount */
> >
> > return method;
> > @@ -565,7 +565,7 @@ build_append_notify_method(GArray *device, const char
> > *name,
> >
> > for (i = 0; i < count; i++) {
> > GArray *target = build_alloc_array();
> > -build_append_nameseg(target, format, i);
> > +build_append_namestrin
Re: [Qemu-devel] [PATCHv3 0/2] Fix virtio-serial migration on bi-endian targets
On 19.12.14 04:57, David Gibson wrote: > On a bi-endian target, with a guest in the non-default endian mode, > attempting to migrate twice in a row with a virtio-serial device wil > cause a qemu SEGV on the second outgoing migration. > > The problem is that virtio_serial_save_device() (and other places) expect > VirtIOSerial->config to be in current guest endianness. On a fresh boot, > virtio_serial_device_realize() will initialize VirtIOSerial->config in > default endianness. It's assumed the guest OS will make its true > endianness known before the device is reset and initialized, then > vser_reset adjusts VirtIOSerial->config into the new endianness. > > But on an incoming migration, the device isn't reset (after all the guest > has a running driver as far as it's concerned), which means that > VirtIOSerial->config retains its default endianness value from > virtio_serial_device_realize(). > > On a subsequent outgoing migration, virtio_serial_save_device() attempts > to interpret VirtIOSerial->config.max_nr_ports in current endianness when > its actually in default endianness and then runs off the end of the > ports_map array in the loop immediately afterwards. > > We could fix this by adjusting VirtIOSerial->config into the correct > current endianness after an incoming migration. But in fact we > already have a host endian copy of the max number of ports readily > available, so it's simpler to just use that instead. Patch 1/2 does > that. > > Once that's done, it becomes clear that there's really no reason to > keep the guest-endian copy of the config space around persistently > (config accesses aren't a fast path, so it can be constructed when > necessary). Patch 2/2 makes that cleanup. Reviewed-by: Alexander Graf
Re: [Qemu-devel] [PATCH 0/2] tcg: Move TCG arch-specific initialization inside TCG code
On 19/12/2014 10:47, Andreas Färber wrote: > How do you imagine this to work with multiple CPU types? From the looks > of it, this is for the target CPU, not the TCG architecture, so > tcg_arch_init() limits us to one implementation unlike now. > On the other hand it doesn't seem to be a per-CPU, i.e. CPUClass, > initialization either... I think these should go in a separate CPUClass method that is called by tcg_exec_init (only for first_cpu now; later for all CPUs if each CPU gets a separate tcg_ctx). Paolo
Re: [Qemu-devel] [PATCH 9/9] target-ppc: Introduce Privileged TM Noops
On Thu, 12/18 10:34, Tom Musta wrote:
> Add the supervisory Transactional Memory instructions treclaim. and
> trechkpt. The implementation is a degenerate one that simply
> checks privileged state, TM availability and then sets CR[0] to
> 0b, just like the unprivileged noops.
And also s-o-b for this :)
Fam
> ---
> target-ppc/translate.c | 38 ++
> 1 files changed, 38 insertions(+), 0 deletions(-)
>
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index a3c79a6..b4a4297 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -9691,6 +9691,40 @@ static void gen_tcheck(DisasContext *ctx)
> tcg_gen_movi_i32(cpu_crf[crfD(ctx->opcode)], 0x8);
> }
>
> +#if defined(CONFIG_USER_ONLY)
> +#define GEN_TM_PRIV_NOOP(name) \
> +static inline void gen_##name(DisasContext *ctx) \
> +{ \
> +gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); \
> +}
> +
> +#else
> +
> +#define GEN_TM_PRIV_NOOP(name) \
> +static inline void gen_##name(DisasContext *ctx) \
> +{ \
> +if (unlikely(ctx->pr)) { \
> +gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); \
> +return;\
> +} \
> +if (unlikely(!ctx->tm_enabled)) { \
> +gen_exception_err(ctx, POWERPC_EXCP_FU, FSCR_IC_TM); \
> +return;\
> +} \
> +/* Because tbegin always fails, the implementation is \
> + * simple: \
> + * \
> + * CR[0] = 0b0 || MSR[TS] || 0b0 \
> + * = 0b0 || 0b00 | 0b0 \
> + */\
> +tcg_gen_movi_i32(cpu_crf[0], 0); \
> +}
> +
> +#endif
> +
> +GEN_TM_PRIV_NOOP(treclaim);
> +GEN_TM_PRIV_NOOP(trechkpt);
> +
> static opcode_t opcodes[] = {
> GEN_HANDLER(invalid, 0x00, 0x00, 0x00, 0x, PPC_NONE),
> GEN_HANDLER(cmp, 0x1F, 0x00, 0x00, 0x0040, PPC_INTEGER),
> @@ -11122,6 +11156,10 @@ GEN_HANDLER2_E(tsr, "tsr", 0x1F, 0x0E, 0x17,
> 0x03DFF800, \
> PPC_NONE, PPC2_TM),
> GEN_HANDLER2_E(tcheck, "tcheck", 0x1F, 0x0E, 0x16, 0x007FF800, \
> PPC_NONE, PPC2_TM),
> +GEN_HANDLER2_E(treclaim, "treclaim", 0x1F, 0x0E, 0x1D, 0x03E0F800, \
> + PPC_NONE, PPC2_TM),
> +GEN_HANDLER2_E(trechkpt, "trechkpt", 0x1F, 0x0E, 0x1F, 0x03FFF800, \
> + PPC_NONE, PPC2_TM),
> };
>
> #include "helper_regs.h"
> --
> 1.7.1
>
>
Re: [Qemu-devel] [Qemu-ppc] Unable to loadvm on qemu-system-ppc -M g3beige (keyboard freeze)
On 18/12/14 23:01, Alexander Graf wrote: > On 18.12.14 22:36, Mark Cave-Ayland wrote: >> On 18/12/14 15:13, Peter Maydell wrote: >> >>> On 18 December 2014 at 14:46, Alexander Graf wrote: On 18.12.14 14:54, Mark Cave-Ayland wrote: > So it looks like several of the device MemoryRegions aren't being added > after the "loadvm". Does this mean there is an object lifecycle issue > here in that some of the initialisation needs to be done in realizefn > rather than initfn or vice-versa? I always thought we're going through both - initfn and realizefn with normal system boot as well as vmstate load. >>> >>> Yes. Migration incoming and vmstate load both work as "create, >>> initialize, realize and reset system as normal, then do state >>> load before running it". >>> So that means that the additional mappings above must be runtime creations that aren't saved and restored properly. >>> >>> Looks likely. Memory regions themselves don't have any saved or >>> reloaded state, so it's the responsibility of the devices that >>> create and control them to ensure that they're set up correctly >>> again on state load. This is trivial for most devices which >>> just have an unchanging set, but controller chip equivalents >>> that allow the guest to map and unmap stuff need to be cleverer. >> >> I think that the problem is that the macio device doesn't have any >> declared vmstate - presumably since no vmstate is saved for that device >> then it doesn't appear in the loadvm restore list and so the object is >> never re-initialised. >> >> I can probably have a go at trying to sort out the VMStateDescription >> (and maybe see how easy it is to convert some of these things to QOM) >> but it seems that MACIOState has an array of qemu_irqs embedded directly >> in it which I'm not sure how to handle. >> >> Can anyone point me towards an example device the best current practice >> for either wiring up qemu_irqs via QOM or how to represent them within a >> VMStateDescription? In general it seems that SPARC and PPC mostly use >> old APIs so there is a distinct lack of good reference examples for >> devices I am familiar with. > > Why exactly would you need to wire up the qemu_irqs? If the lines are > asserted at the point of migration, the MPIC model should migrate over > the fact that a line is pending, no? It seems that I've misunderstood something mentioned earlier in the thread, i.e. that loadvm also does the create, initialize, realize and reset cycle. At least issuing "loadvm" in the monitor doesn't seem to do that as far as I can see here? Otherwise I could see how recreating qemu_irqs via the old-style init() functions every time "loadvm" is issued would cause quite a bit of chaos. Taking a different approach with a debugger this morning I think I may have just figured out what's going on with the macio device - more to follow soon I hope. ATB, Mark.
[Qemu-devel] broken backward migration due to commit 461a275 "parallel: adding vmstate for save/restore"
steps to reproduce: Current master source: qemu-system-x86_64 -monitor stdio -M pc-i440fx-2.1 (qemu) stop (qemu) migrate "exec:gzip -c > STATEFILE.gz" Target: qemu-system-x86_64-2.1 -monitor stdio -M pc-i440fx-2.1 -incoming "exec: gzip -c -d STATEFILE.gz"
Re: [Qemu-devel] broken backward migration due to commit 461a275 "parallel: adding vmstate for save/restore"
On 19/12/2014 12:03, Igor Mammedov wrote: > steps to reproduce: > > Current master source: > qemu-system-x86_64 -monitor stdio -M pc-i440fx-2.1 > (qemu) stop > (qemu) migrate "exec:gzip -c > STATEFILE.gz" > > Target: > qemu-system-x86_64-2.1 -monitor stdio -M pc-i440fx-2.1 -incoming "exec: > gzip -c -d STATEFILE.gz" > That's expected. Backwards migration is not supported by upstream, and there's no equivalent of subsections for entire devices. Just use "-parallel none". Paolo
Re: [Qemu-devel] [PATCH 5/8] target-i386: Set APIC ID using cpu_index on CONFIG_USER
On 19/12/2014 03:41, Eduardo Habkost wrote:
> +object_property_set_int(OBJECT(cpu), CPU(cpu)->cpu_index, "apic-id",
> +&error);
> +if (error) {
> +goto out;
> +}
> +
Should this use &error_abort?
Paolo
Re: [Qemu-devel] [PATCH 4/8] target-i386: Keep track of apic-id setting
On 19/12/2014 03:41, Eduardo Habkost wrote:
> Set a flag indicating that the apic-id property was set, so we can later
> ensure we won't try to realize a X86CPU object before apic-id was set.
>
> Signed-off-by: Eduardo Habkost
> Cc: Gu Zheng
> ---
> target-i386/cpu-qom.h | 1 +
> target-i386/cpu.c | 3 ++-
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
> index 4a6f48a..ba0ee15 100644
> --- a/target-i386/cpu-qom.h
> +++ b/target-i386/cpu-qom.h
> @@ -94,6 +94,7 @@ typedef struct X86CPU {
> bool migratable;
> bool host_features;
> uint32_t apic_id;
> +bool apic_id_set;
>
> /* if true the CPUID code directly forward host cache leaves to the
> guest */
> bool cache_info_passthrough;
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index cbed717..bb9525d 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1691,7 +1691,7 @@ static void x86_cpuid_get_apic_id(Object *obj, Visitor
> *v, void *opaque,
>const char *name, Error **errp)
> {
> X86CPU *cpu = X86_CPU(obj);
> -int64_t value = cpu->apic_id;
> +int64_t value = cpu->apic_id_set ? cpu->apic_id : -1;
Should this return an error if the apic_id was not set?
Paolo
> visit_type_int(v, &value, name, errp);
> }
> @@ -1729,6 +1729,7 @@ static void x86_cpuid_set_apic_id(Object *obj, Visitor
> *v, void *opaque,
> return;
> }
> cpu->apic_id = value;
> +cpu->apic_id_set = true;
> }
>
> /* Generic getter for "feature-words" and "filtered-features" properties */
>
Re: [Qemu-devel] [PATCH 7/8] target-i386: Move topology.h to hw/i386/topology.h
On 19/12/2014 03:41, Eduardo Habkost wrote:
> This will allow the PC code to use the header, and lets us eliminate the
> QEMU_INCLUDES hack inside tests/Makefile.
>
> Signed-off-by: Eduardo Habkost
Please use include/hw/i386/topology.h (the toplevel build and source
directories should not be part of the #include path, so it's a bug that
this works).
Paolo
> ---
> {target-i386 => hw/i386}/topology.h | 6 +++---
> target-i386/cpu.c | 2 +-
> tests/Makefile | 2 --
> tests/test-x86-cpuid.c | 2 +-
> 4 files changed, 5 insertions(+), 7 deletions(-)
> rename {target-i386 => hw/i386}/topology.h (97%)
>
> diff --git a/target-i386/topology.h b/hw/i386/topology.h
> similarity index 97%
> rename from target-i386/topology.h
> rename to hw/i386/topology.h
> index 07a6c5f..9c6f3a9 100644
> --- a/target-i386/topology.h
> +++ b/hw/i386/topology.h
> @@ -21,8 +21,8 @@
> * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> * THE SOFTWARE.
> */
> -#ifndef TARGET_I386_TOPOLOGY_H
> -#define TARGET_I386_TOPOLOGY_H
> +#ifndef HW_I386_TOPOLOGY_H
> +#define HW_I386_TOPOLOGY_H
>
> /* This file implements the APIC-ID-based CPU topology enumeration logic,
> * documented at the following document:
> @@ -131,4 +131,4 @@ static inline apic_id_t x86_apicid_from_cpu_idx(unsigned
> nr_cores,
> return apicid_from_topo_ids(nr_cores, nr_threads, pkg_id, core_id,
> smt_id);
> }
>
> -#endif /* TARGET_I386_TOPOLOGY_H */
> +#endif /* HW_I386_TOPOLOGY_H */
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 4b6e19b..d8cd7c9 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -25,7 +25,7 @@
> #include "sysemu/kvm.h"
> #include "sysemu/cpus.h"
> #include "kvm_i386.h"
> -#include "topology.h"
> +#include "hw/i386/topology.h"
>
> #include "qemu/option.h"
> #include "qemu/config-file.h"
> diff --git a/tests/Makefile b/tests/Makefile
> index e4ddb6a..bdc7cc5 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -232,8 +232,6 @@ $(test-obj-y): QEMU_INCLUDES += -Itests
> QEMU_CFLAGS += -I$(SRC_PATH)/tests
> qom-core-obj = qom/object.o qom/qom-qobject.o qom/container.o
>
> -tests/test-x86-cpuid.o: QEMU_INCLUDES += -I$(SRC_PATH)/target-i386
> -
> tests/check-qint$(EXESUF): tests/check-qint.o libqemuutil.a
> tests/check-qstring$(EXESUF): tests/check-qstring.o libqemuutil.a
> tests/check-qdict$(EXESUF): tests/check-qdict.o libqemuutil.a
> diff --git a/tests/test-x86-cpuid.c b/tests/test-x86-cpuid.c
> index 8d9f96a..6cd20d4 100644
> --- a/tests/test-x86-cpuid.c
> +++ b/tests/test-x86-cpuid.c
> @@ -24,7 +24,7 @@
>
> #include
>
> -#include "topology.h"
> +#include "hw/i386/topology.h"
>
> static void test_topo_bits(void)
> {
>
Re: [Qemu-devel] [PATCH V3] net: don't use set/get_pointer() in set/get_netdev()
On Mon, Oct 13, 2014 at 01:16:37PM +0800, Jason Wang wrote: > Commit 1ceef9f27359cbe92ef124bf74de6f792e71f6fb (net: multiqueue > support) tries to use set_pointer() and get_pointer() to set and get > NICPeers which is not a pointer defined in DEFINE_PROP_NETDEV. This > trick works but result a unclean and fragile implementation (e.g > print_netdev and parse_netdev). > > This patch solves this issue by not using set/get_pinter() and set and > get netdev directly in set_netdev() and get_netdev(). After this the > parse_netdev() and print_netdev() were no longer used and dropped from > the source. > > Cc: Markus Armbruster > Cc: Stefan Hajnoczi > Cc: Peter Maydell > Signed-off-by: Jason Wang > --- > Changes from V2: > - Use error_setg() instead of error_set_from_qdev_prop_error() for E2BIG > error. > - Clean the return part of the set_netdev() since > eror_set_from_qdev_prop_error() does nothing when err is 0. > Changes from V1: > - validate ncs pointer before accessing them, this fixes the qtest failure > on arm. > --- > hw/core/qdev-properties-system.c | 70 > ++-- > 1 file changed, 38 insertions(+), 32 deletions(-) Renamed 'err' label to 'out' as suggested by Markus. Thanks, applied to my net tree: https://github.com/stefanha/qemu/commits/net Stefan pgp0XAYGLk80L.pgp Description: PGP signature
Re: [Qemu-devel] broken backward migration due to commit 461a275 "parallel: adding vmstate for save/restore"
On Fri, 19 Dec 2014 12:15:20 +0100 Paolo Bonzini wrote: > > > On 19/12/2014 12:03, Igor Mammedov wrote: > > steps to reproduce: > > > > Current master source: > > qemu-system-x86_64 -monitor stdio -M pc-i440fx-2.1 > > (qemu) stop > > (qemu) migrate "exec:gzip -c > STATEFILE.gz" > > > > Target: > > qemu-system-x86_64-2.1 -monitor stdio -M pc-i440fx-2.1 -incoming "exec: > > gzip -c -d STATEFILE.gz" > > > > That's expected. Backwards migration is not supported by upstream, and > there's no equivalent of subsections for entire devices. > > Just use "-parallel none". ok There is one more commit that breaks it, this time with subsection 6c3bff0 "exec: Save CPUState::exception_index field" qemu: warning: error while loading state for instance 0x0 of device 'cpu_common' the same reproducer with -parallel none > > Paolo
[Qemu-devel] [PATCH v3] target-tricore: pretty-print register dump and show more status registers
Now using psw_read() to retrieve the status bits correctly.
Signed-off-by: Alex Zuepke
---
target-tricore/translate.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index e3eeedb..3d87346 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -85,22 +85,31 @@ void tricore_cpu_dump_state(CPUState *cs, FILE *f,
{
TriCoreCPU *cpu = TRICORE_CPU(cs);
CPUTriCoreState *env = &cpu->env;
+uint32_t psw;
int i;
-cpu_fprintf(f, "PC=%08x\n", env->PC);
+psw = psw_read(env);
+
+cpu_fprintf(f, "PC: " TARGET_FMT_lx, env->PC);
+cpu_fprintf(f, " PSW: " TARGET_FMT_lx, psw);
+cpu_fprintf(f, " ICR: " TARGET_FMT_lx, env->ICR);
+cpu_fprintf(f, "\nPCXI: " TARGET_FMT_lx, env->PCXI);
+cpu_fprintf(f, " FCX: " TARGET_FMT_lx, env->FCX);
+cpu_fprintf(f, " LCX: " TARGET_FMT_lx, env->LCX);
+
for (i = 0; i < 16; ++i) {
if ((i & 3) == 0) {
-cpu_fprintf(f, "GPR A%02d:", i);
+cpu_fprintf(f, "\nGPR A%02d:", i);
}
-cpu_fprintf(f, " %s " TARGET_FMT_lx, regnames_a[i], env->gpr_a[i]);
+cpu_fprintf(f, " " TARGET_FMT_lx, env->gpr_a[i]);
}
for (i = 0; i < 16; ++i) {
if ((i & 3) == 0) {
-cpu_fprintf(f, "GPR D%02d:", i);
+cpu_fprintf(f, "\nGPR D%02d:", i);
}
-cpu_fprintf(f, " %s " TARGET_FMT_lx, regnames_d[i], env->gpr_d[i]);
+cpu_fprintf(f, " " TARGET_FMT_lx, env->gpr_d[i]);
}
-
+cpu_fprintf(f, "\n");
}
/*
--
1.7.9.5
Re: [Qemu-devel] broken backward migration due to commit 461a275 "parallel: adding vmstate for save/restore"
On Fri, 19 Dec 2014 12:32:52 +0100 Igor Mammedov wrote: > On Fri, 19 Dec 2014 12:15:20 +0100 > Paolo Bonzini wrote: > > > > > > > On 19/12/2014 12:03, Igor Mammedov wrote: > > > steps to reproduce: > > > > > > Current master source: > > > qemu-system-x86_64 -monitor stdio -M pc-i440fx-2.1 > > > (qemu) stop > > > (qemu) migrate "exec:gzip -c > STATEFILE.gz" > > > > > > Target: > > > qemu-system-x86_64-2.1 -monitor stdio -M pc-i440fx-2.1 -incoming "exec: > > > gzip -c -d STATEFILE.gz" > > > > > > > That's expected. Backwards migration is not supported by upstream, and > > there's no equivalent of subsections for entire devices. > > > > Just use "-parallel none". > ok > > There is one more commit that breaks it, this time with subsection > 6c3bff0 "exec: Save CPUState::exception_index field" > > qemu: warning: error while loading state for instance 0x0 of device > 'cpu_common' > the same reproducer with -parallel none correction this time it's with -M pc-i440fx-1.7 and target qemu-1.7 > > > > > Paolo > >
Re: [Qemu-devel] [PATCH] block-migration: fix pending() return value
Hi there.. Can someone review my bug fix? I'll surely fix typos in description after it. Best regards, Vladimir On 11.12.2014 18:07, Eric Blake wrote: On 12/11/2014 04:55 AM, Vladimir Sementsov-Ogievskiy wrote: Because of wrong return value of .save_live_pending() in block-migration, migration finishes before the whole disk is transferred. Such situation occures when the migration s/occures/occurs/ process is fast enouth, for example when source and dest s/enouth/enough/ are on the same host. If in the bulk phase we return something < max_size, we will skip transferring the tail of the device. Currently we have "set pending to BLOCK_SIZE if it is zero" for bulk phase, but there no guarantee, that it will be < max_size. True approach is to return, for example, max_size+1 when we are in the bulk phase. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block-migration.c | 4 ++-- I'll let someone else review the code proper.
[Qemu-devel] [PATCH v3 2/8] pc: acpi: decribe bridge device as not hotpluggable
when bridge hotplug is disabled, i.e. for machine
types less then 2.0, bridge device was created as
hotpluggable by mistake since commit 133a2da (2.1).
Fix it by just creating it as a present device as
it was done in 1.7.
Signed-off-by: Igor Mammedov
---
hw/i386/acpi-build.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index a4d0c0c..c151fde 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -919,7 +919,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
}
}
-if (!dc->hotpluggable || bridge_in_acpi) {
+if (!dc->hotpluggable || pc->is_bridge) {
clear_bit(slot, slot_hotplug_enable);
}
}
--
1.8.3.1
[Qemu-devel] [PATCH v3 5/8] acpi: move generic aml building helpers into dedictated file
the will be later used for composing AML primitives
and all that could be reused later for ARM machines
as well.
Signed-off-by: Igor Mammedov
---
v2:
fix wrong ident in moved code
---
hw/acpi/Makefile.objs| 1 +
hw/acpi/acpi_gen_utils.c | 166 +++
hw/i386/acpi-build.c | 162 +-
include/hw/acpi/acpi_gen_utils.h | 23 ++
4 files changed, 192 insertions(+), 160 deletions(-)
create mode 100644 hw/acpi/acpi_gen_utils.c
create mode 100644 include/hw/acpi/acpi_gen_utils.h
diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
index acd2389..4237232 100644
--- a/hw/acpi/Makefile.objs
+++ b/hw/acpi/Makefile.objs
@@ -1,3 +1,4 @@
common-obj-$(CONFIG_ACPI) += core.o piix4.o ich9.o pcihp.o cpu_hotplug.o
common-obj-$(CONFIG_ACPI) += memory_hotplug.o
common-obj-$(CONFIG_ACPI) += acpi_interface.o
+common-obj-$(CONFIG_ACPI) += acpi_gen_utils.o
diff --git a/hw/acpi/acpi_gen_utils.c b/hw/acpi/acpi_gen_utils.c
new file mode 100644
index 000..5f64c37
--- /dev/null
+++ b/hw/acpi/acpi_gen_utils.c
@@ -0,0 +1,166 @@
+#include
+#include
+#include
+#include
+#include "hw/acpi/acpi_gen_utils.h"
+
+GArray *build_alloc_array(void)
+{
+return g_array_new(false, true /* clear */, 1);
+}
+
+void build_free_array(GArray *array)
+{
+g_array_free(array, true);
+}
+
+void build_prepend_byte(GArray *array, uint8_t val)
+{
+g_array_prepend_val(array, val);
+}
+
+void build_append_byte(GArray *array, uint8_t val)
+{
+g_array_append_val(array, val);
+}
+
+void build_append_array(GArray *array, GArray *val)
+{
+g_array_append_vals(array, val->data, val->len);
+}
+
+#define ACPI_NAMESEG_LEN 4
+
+void GCC_FMT_ATTR(2, 3)
+build_append_nameseg(GArray *array, const char *format, ...)
+{
+/* It would be nicer to use g_string_vprintf but it's only there in 2.22 */
+char s[] = "";
+int len;
+va_list args;
+
+va_start(args, format);
+len = vsnprintf(s, sizeof s, format, args);
+va_end(args);
+
+assert(len <= ACPI_NAMESEG_LEN);
+
+g_array_append_vals(array, s, len);
+/* Pad up to ACPI_NAMESEG_LEN characters if necessary. */
+g_array_append_vals(array, "", ACPI_NAMESEG_LEN - len);
+}
+
+/* 5.4 Definition Block Encoding */
+enum {
+PACKAGE_LENGTH_1BYTE_SHIFT = 6, /* Up to 63 - use extra 2 bits. */
+PACKAGE_LENGTH_2BYTE_SHIFT = 4,
+PACKAGE_LENGTH_3BYTE_SHIFT = 12,
+PACKAGE_LENGTH_4BYTE_SHIFT = 20,
+};
+
+void build_prepend_package_length(GArray *package, unsigned min_bytes)
+{
+uint8_t byte;
+unsigned length = package->len;
+unsigned length_bytes;
+
+if (length + 1 < (1 << PACKAGE_LENGTH_1BYTE_SHIFT)) {
+length_bytes = 1;
+} else if (length + 2 < (1 << PACKAGE_LENGTH_3BYTE_SHIFT)) {
+length_bytes = 2;
+} else if (length + 3 < (1 << PACKAGE_LENGTH_4BYTE_SHIFT)) {
+length_bytes = 3;
+} else {
+length_bytes = 4;
+}
+
+/* Force length to at least min_bytes.
+ * This wastes memory but that's how bios did it.
+ */
+length_bytes = MAX(length_bytes, min_bytes);
+
+/* PkgLength is the length of the inclusive length of the data. */
+length += length_bytes;
+
+switch (length_bytes) {
+case 1:
+byte = length;
+build_prepend_byte(package, byte);
+return;
+case 4:
+byte = length >> PACKAGE_LENGTH_4BYTE_SHIFT;
+build_prepend_byte(package, byte);
+length &= (1 << PACKAGE_LENGTH_4BYTE_SHIFT) - 1;
+/* fall through */
+case 3:
+byte = length >> PACKAGE_LENGTH_3BYTE_SHIFT;
+build_prepend_byte(package, byte);
+length &= (1 << PACKAGE_LENGTH_3BYTE_SHIFT) - 1;
+/* fall through */
+case 2:
+byte = length >> PACKAGE_LENGTH_2BYTE_SHIFT;
+build_prepend_byte(package, byte);
+length &= (1 << PACKAGE_LENGTH_2BYTE_SHIFT) - 1;
+/* fall through */
+}
+/*
+ * Most significant two bits of byte zero indicate how many following bytes
+ * are in PkgLength encoding.
+ */
+byte = ((length_bytes - 1) << PACKAGE_LENGTH_1BYTE_SHIFT) | length;
+build_prepend_byte(package, byte);
+}
+
+void build_package(GArray *package, uint8_t op, unsigned min_bytes)
+{
+build_prepend_package_length(package, min_bytes);
+build_prepend_byte(package, op);
+}
+
+void build_extop_package(GArray *package, uint8_t op)
+{
+build_package(package, op, 1);
+build_prepend_byte(package, 0x5B); /* ExtOpPrefix */
+}
+
+void build_append_value(GArray *table, uint32_t value, int size)
+{
+uint8_t prefix;
+int i;
+
+switch (size) {
+case 1:
+prefix = 0x0A; /* BytePrefix */
+break;
+case 2:
+prefix = 0x0B; /* WordPrefix */
+break;
+case 4:
+prefix = 0x0C; /* DWordPrefix */
+break;
+default:
+assert(0);
+return;
+}
+build_append_byte(table, prefix);
+
[Qemu-devel] [PATCH v3 0/8] pc: acpi: various fixes and cleanups
changes from v2: * codding style fixups * check for SegCount earlier * use hotpluggable device object instead of not hotpluggable for non present devices, and add it only when bus itself is hotpluggable changes from v1: * drop: [PATCH 7/9] acpi: replace opencoded notify codes with named values * use Michael's suggestion to improve build_append_nameseg() * drop long scope names and go back to recursion, but still significantly simplify building of PCI tree this series is an attempt to shave off a bunch of not directly related patches from already big dynamic AML series (although it's dependency for it) Tested: on XPsp3 to WS2012R2 and REHL6/7 guests. Git tree for testing: https://github.com/imammedo/qemu/commits/acpi_pci_gen_simplification_v2 Igor Mammedov (8): pc: acpi: fix WindowsXP BSOD when memory hotplug is enabled pc: acpi: decribe bridge device as not hotpluggable pc: acpi-build: cleanup AcpiPmInfo initialization acpi: build_append_nameseg(): add padding if necessary acpi: move generic aml building helpers into dedictated file acpi: add build_append_namestring() helper acpi: drop min-bytes in build_package() pc: acpi-build: simplify PCI bus tree generation hw/acpi/Makefile.objs | 1 + hw/acpi/acpi_gen_utils.c | 248 + hw/i386/acpi-build.c | 454 -- hw/i386/acpi-dsdt-cpu-hotplug.dsl | 1 + include/hw/acpi/acpi_gen_utils.h | 23 ++ 5 files changed, 366 insertions(+), 361 deletions(-) create mode 100644 hw/acpi/acpi_gen_utils.c create mode 100644 include/hw/acpi/acpi_gen_utils.h -- 1.8.3.1
[Qemu-devel] [PATCH v3 3/8] pc: acpi-build: cleanup AcpiPmInfo initialization
zero initialize AcpiPmInfo struct to reduce code bloat
a little bit.
Signed-off-by: Igor Mammedov
Reviewed-by: Claudio Fontana
---
hw/i386/acpi-build.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index c151fde..0351363 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -169,6 +169,8 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
Object *obj = NULL;
QObject *o;
+memset(pm, 0, sizeof(*pm));
+
if (piix) {
obj = piix;
}
@@ -181,22 +183,16 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
o = object_property_get_qobject(obj, ACPI_PM_PROP_S3_DISABLED, NULL);
if (o) {
pm->s3_disabled = qint_get_int(qobject_to_qint(o));
-} else {
-pm->s3_disabled = false;
}
qobject_decref(o);
o = object_property_get_qobject(obj, ACPI_PM_PROP_S4_DISABLED, NULL);
if (o) {
pm->s4_disabled = qint_get_int(qobject_to_qint(o));
-} else {
-pm->s4_disabled = false;
}
qobject_decref(o);
o = object_property_get_qobject(obj, ACPI_PM_PROP_S4_VAL, NULL);
if (o) {
pm->s4_val = qint_get_int(qobject_to_qint(o));
-} else {
-pm->s4_val = false;
}
qobject_decref(o);
--
1.8.3.1
[Qemu-devel] [PATCH v3 1/8] pc: acpi: fix WindowsXP BSOD when memory hotplug is enabled
ACPI parser in XP considers PNP0A06 devices of CPU and
memory hotplug as duplicates. Adding unique _UID
to CPU hotplug device fixes BSOD.
Signed-off-by: Igor Mammedov
---
hw/i386/acpi-dsdt-cpu-hotplug.dsl | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/i386/acpi-dsdt-cpu-hotplug.dsl
b/hw/i386/acpi-dsdt-cpu-hotplug.dsl
index 34aab5a..268d870 100644
--- a/hw/i386/acpi-dsdt-cpu-hotplug.dsl
+++ b/hw/i386/acpi-dsdt-cpu-hotplug.dsl
@@ -94,6 +94,7 @@ Scope(\_SB) {
Device(CPU_HOTPLUG_RESOURCE_DEVICE) {
Name(_HID, EisaId("PNP0A06"))
+Name(_UID, "CPU hotplug resources")
Name(_CRS, ResourceTemplate() {
IO(Decode16, CPU_STATUS_BASE, CPU_STATUS_BASE, 0, CPU_STATUS_LEN)
--
1.8.3.1
[Qemu-devel] [PATCH v3 6/8] acpi: add build_append_namestring() helper
Use build_append_namestring() instead of build_append_nameseg()
So user won't have to care whether name is NameSeg, NamePath or
NameString.
See for refernce ACPI 5.0: 20.2.2 Name Objects Encoding
Signed-off-by: Igor Mammedov
---
v2:
assert on wrong Segcount earlier and extend condition to
seg_count <= 0xFF || seg_count < 1
---
hw/acpi/acpi_gen_utils.c | 90 +++-
hw/i386/acpi-build.c | 35 +++-
include/hw/acpi/acpi_gen_utils.h | 2 +-
3 files changed, 106 insertions(+), 21 deletions(-)
diff --git a/hw/acpi/acpi_gen_utils.c b/hw/acpi/acpi_gen_utils.c
index 5f64c37..d5fca8e 100644
--- a/hw/acpi/acpi_gen_utils.c
+++ b/hw/acpi/acpi_gen_utils.c
@@ -31,7 +31,7 @@ void build_append_array(GArray *array, GArray *val)
#define ACPI_NAMESEG_LEN 4
-void GCC_FMT_ATTR(2, 3)
+static void GCC_FMT_ATTR(2, 3)
build_append_nameseg(GArray *array, const char *format, ...)
{
/* It would be nicer to use g_string_vprintf but it's only there in 2.22 */
@@ -50,6 +50,94 @@ build_append_nameseg(GArray *array, const char *format, ...)
g_array_append_vals(array, "", ACPI_NAMESEG_LEN - len);
}
+static const uint8_t ACPI_DualNamePrefix = 0x2E;
+static const uint8_t ACPI_MultiNamePrefix = 0x2F;
+
+static void
+build_append_namestringv(GArray *array, const char *format, va_list ap)
+{
+/* It would be nicer to use g_string_vprintf but it's only there in 2.22 */
+char *s;
+int len;
+va_list va_len;
+char **segs;
+char **segs_iter;
+int seg_count = 0;
+
+va_copy(va_len, ap);
+len = vsnprintf(NULL, 0, format, va_len);
+va_end(va_len);
+len += 1;
+s = g_new(typeof(*s), len);
+
+len = vsnprintf(s, len, format, ap);
+
+segs = g_strsplit(s, ".", 0);
+g_free(s);
+
+/* count segments */
+segs_iter = segs;
+while (*segs_iter) {
+++segs_iter;
+++seg_count;
+}
+/*
+ * ACPI 5.0 spec: 20.2.2 Name Objects Encoding:
+ * "SegCount can be from 1 to 255"
+ */
+assert(seg_count <= 0xFF || seg_count < 1);
+
+/* handle RootPath || PrefixPath */
+s = *segs;
+while (*s == '\\' || *s == '^') {
+g_array_append_val(array, *s);
+++s;
+}
+
+switch (seg_count) {
+case 1:
+if (!*s) { /* NullName */
+const uint8_t nullpath = 0;
+g_array_append_val(array, nullpath);
+} else {
+build_append_nameseg(array, "%s", s);
+}
+break;
+
+case 2:
+g_array_append_val(array, ACPI_DualNamePrefix);
+build_append_nameseg(array, "%s", s);
+build_append_nameseg(array, "%s", segs[1]);
+break;
+
+default:
+g_array_append_val(array, ACPI_MultiNamePrefix);
+g_array_append_val(array, seg_count);
+
+/* handle the 1st segment manually due to prefix/root path */
+build_append_nameseg(array, "%s", s);
+
+/* add the rest of segments */
+segs_iter = segs + 1;
+while (*segs_iter) {
+build_append_nameseg(array, "%s", *segs_iter);
+++segs_iter;
+}
+break;
+}
+g_strfreev(segs);
+}
+
+void GCC_FMT_ATTR(2, 3)
+build_append_namestring(GArray *array, const char *format, ...)
+{
+va_list ap;
+
+va_start(ap, format);
+build_append_namestringv(array, format, ap);
+va_end(ap);
+}
+
/* 5.4 Definition Block Encoding */
enum {
PACKAGE_LENGTH_1BYTE_SHIFT = 6, /* Up to 63 - use extra 2 bits. */
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index f7282ef..7642f6d 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -279,7 +279,7 @@ static GArray *build_alloc_method(const char *name, uint8_t
arg_count)
{
GArray *method = build_alloc_array();
-build_append_nameseg(method, "%s", name);
+build_append_namestring(method, "%s", name);
build_append_byte(method, arg_count); /* MethodFlags: ArgCount */
return method;
@@ -571,7 +571,7 @@ build_append_notify_method(GArray *device, const char *name,
for (i = 0; i < count; i++) {
GArray *target = build_alloc_array();
-build_append_nameseg(target, format, i);
+build_append_namestring(target, format, i);
assert(i < 256); /* Fits in 1 byte */
build_append_notify_target_ifequal(method, target, i, 1);
build_free_array(target);
@@ -699,24 +699,24 @@ static void build_pci_bus_end(PCIBus *bus, void
*bus_state)
if (bus->parent_dev) {
op = 0x82; /* DeviceOp */
-build_append_nameseg(bus_table, "S%.02X",
+build_append_namestring(bus_table, "S%.02X",
bus->parent_dev->devfn);
build_append_byte(bus_table, 0x08); /* NameOp */
-build_append_nameseg(bus_table, "_SUN");
+build_append_namestring(bus_table, "_SUN");
build_append_value(bus_table, PCI_SLOT(bus->parent_dev->devfn), 1);
build_append_byte(bus_table, 0x
[Qemu-devel] [PATCH v3 4/8] acpi: build_append_nameseg(): add padding if necessary
According to ACPI spec NameSeg shorter than 4 characters
must be padded up to 4 characters with "_" symbol.
ACPI 5.0: 20.2.2 "Name Objects Encoding"
Do it in build_append_nameseg() so that caller shouldn't know
or care about it.
Signed-off-by: Igor Mammedov
Reviewed-by: Claudio Fontana
---
v2:
* simplify padding, suggested by: "Michael S. Tsirkin"
---
hw/i386/acpi-build.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 0351363..cab3c76 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -298,6 +298,8 @@ static inline void build_append_array(GArray *array, GArray
*val)
g_array_append_vals(array, val->data, val->len);
}
+#define ACPI_NAMESEG_LEN 4
+
static void GCC_FMT_ATTR(2, 3)
build_append_nameseg(GArray *array, const char *format, ...)
{
@@ -310,8 +312,11 @@ build_append_nameseg(GArray *array, const char *format,
...)
len = vsnprintf(s, sizeof s, format, args);
va_end(args);
-assert(len == 4);
+assert(len <= ACPI_NAMESEG_LEN);
+
g_array_append_vals(array, s, len);
+/* Pad up to ACPI_NAMESEG_LEN characters if necessary. */
+g_array_append_vals(array, "", ACPI_NAMESEG_LEN - len);
}
/* 5.4 Definition Block Encoding */
@@ -852,7 +857,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
if (bus->parent_dev) {
op = 0x82; /* DeviceOp */
-build_append_nameseg(bus_table, "S%.02X_",
+build_append_nameseg(bus_table, "S%.02X",
bus->parent_dev->devfn);
build_append_byte(bus_table, 0x08); /* NameOp */
build_append_nameseg(bus_table, "_SUN");
@@ -972,7 +977,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
build_append_int(notify, 0x1U << i);
build_append_byte(notify, 0x00); /* NullName */
build_append_byte(notify, 0x86); /* NotifyOp */
-build_append_nameseg(notify, "S%.02X_", PCI_DEVFN(i, 0));
+build_append_nameseg(notify, "S%.02X", PCI_DEVFN(i, 0));
build_append_byte(notify, 0x69); /* Arg1Op */
/* Pack it up */
@@ -1029,7 +1034,7 @@ static void build_pci_bus_end(PCIBus *bus, void
*bus_state)
if (bus->parent_dev) {
build_append_byte(parent->notify_table, '^'); /* ParentPrefixChar
*/
build_append_byte(parent->notify_table, 0x2E); /* DualNamePrefix */
-build_append_nameseg(parent->notify_table, "S%.02X_",
+build_append_nameseg(parent->notify_table, "S%.02X",
bus->parent_dev->devfn);
build_append_nameseg(parent->notify_table, "PCNT");
}
@@ -1099,7 +1104,7 @@ build_ssdt(GArray *table_data, GArray *linker,
GArray *sb_scope = build_alloc_array();
uint8_t op = 0x10; /* ScopeOp */
-build_append_nameseg(sb_scope, "_SB_");
+build_append_nameseg(sb_scope, "_SB");
/* build Processor object for each processor */
for (i = 0; i < acpi_cpus; i++) {
--
1.8.3.1
[Qemu-devel] [PATCH v3 7/8] acpi: drop min-bytes in build_package()
Signed-off-by: Igor Mammedov
---
hw/acpi/acpi_gen_utils.c | 14 --
hw/i386/acpi-build.c | 13 ++---
include/hw/acpi/acpi_gen_utils.h | 4 ++--
3 files changed, 12 insertions(+), 19 deletions(-)
diff --git a/hw/acpi/acpi_gen_utils.c b/hw/acpi/acpi_gen_utils.c
index d5fca8e..eee8066 100644
--- a/hw/acpi/acpi_gen_utils.c
+++ b/hw/acpi/acpi_gen_utils.c
@@ -146,7 +146,7 @@ enum {
PACKAGE_LENGTH_4BYTE_SHIFT = 20,
};
-void build_prepend_package_length(GArray *package, unsigned min_bytes)
+void build_prepend_package_length(GArray *package)
{
uint8_t byte;
unsigned length = package->len;
@@ -162,11 +162,6 @@ void build_prepend_package_length(GArray *package,
unsigned min_bytes)
length_bytes = 4;
}
-/* Force length to at least min_bytes.
- * This wastes memory but that's how bios did it.
- */
-length_bytes = MAX(length_bytes, min_bytes);
-
/* PkgLength is the length of the inclusive length of the data. */
length += length_bytes;
@@ -199,15 +194,15 @@ void build_prepend_package_length(GArray *package,
unsigned min_bytes)
build_prepend_byte(package, byte);
}
-void build_package(GArray *package, uint8_t op, unsigned min_bytes)
+void build_package(GArray *package, uint8_t op)
{
-build_prepend_package_length(package, min_bytes);
+build_prepend_package_length(package);
build_prepend_byte(package, op);
}
void build_extop_package(GArray *package, uint8_t op)
{
-build_package(package, op, 1);
+build_package(package, op);
build_prepend_byte(package, 0x5B); /* ExtOpPrefix */
}
@@ -251,4 +246,3 @@ void build_append_int(GArray *table, uint32_t value)
build_append_value(table, value, 4);
}
}
-
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 7642f6d..94202b5 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -289,7 +289,7 @@ static void build_append_and_cleanup_method(GArray *device,
GArray *method)
{
uint8_t op = 0x14; /* MethodOp */
-build_package(method, op, 0);
+build_package(method, op);
build_append_array(device, method);
build_free_array(method);
@@ -310,7 +310,7 @@ static void build_append_notify_target_ifequal(GArray
*method,
build_append_byte(notify, 0x69); /* Arg1Op */
/* Pack it up */
-build_package(notify, op, 1);
+build_package(notify, op);
build_append_array(method, notify);
@@ -823,7 +823,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
build_append_byte(notify, 0x69); /* Arg1Op */
/* Pack it up */
-build_package(notify, op, 0);
+build_package(notify, op);
build_append_array(method, notify);
@@ -864,7 +864,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
if (bus->parent_dev) {
build_extop_package(bus_table, op);
} else {
-build_package(bus_table, op, 0);
+build_package(bus_table, op);
}
/* Append our bus description to parent table */
@@ -987,7 +987,7 @@ build_ssdt(GArray *table_data, GArray *linker,
build_append_byte(package, b);
}
-build_package(package, op, 2);
+build_package(package, op);
build_append_array(sb_scope, package);
build_free_array(package);
}
@@ -1035,8 +1035,7 @@ build_ssdt(GArray *table_data, GArray *linker,
build_append_array(sb_scope, hotplug_state.device_table);
build_pci_bus_state_cleanup(&hotplug_state);
}
-
-build_package(sb_scope, op, 3);
+build_package(sb_scope, op);
build_append_array(table_data, sb_scope);
build_free_array(sb_scope);
}
diff --git a/include/hw/acpi/acpi_gen_utils.h b/include/hw/acpi/acpi_gen_utils.h
index fd50625..199f003 100644
--- a/include/hw/acpi/acpi_gen_utils.h
+++ b/include/hw/acpi/acpi_gen_utils.h
@@ -14,8 +14,8 @@ void build_append_array(GArray *array, GArray *val);
void GCC_FMT_ATTR(2, 3)
build_append_namestring(GArray *array, const char *format, ...);
-void build_prepend_package_length(GArray *package, unsigned min_bytes);
-void build_package(GArray *package, uint8_t op, unsigned min_bytes);
+void build_prepend_package_length(GArray *package);
+void build_package(GArray *package, uint8_t op);
void build_append_value(GArray *table, uint32_t value, int size);
void build_append_int(GArray *table, uint32_t value);
void build_extop_package(GArray *package, uint8_t op);
--
1.8.3.1
[Qemu-devel] [PATCH v3 8/8] pc: acpi-build: simplify PCI bus tree generation
it basicaly does the same as original approach,
* just without bus/notify tables tracking (less obscure)
which is easier to follow.
* drops unnecessary loops and bitmaps,
creating devices and notification method in the same loop.
* saves us ~100LOC
change in behavior:
* generate hotpluggable device entries for empty slots
only if BUS itself is hotpluggable, otherwise do not
create them.
Signed-off-by: Igor Mammedov
---
v3:
* use hotpluggable device object instead of not hotpluggable
for non present devices, and add it only when bus itself is
hotpluggable
---
hw/i386/acpi-build.c | 265 +++
1 file changed, 79 insertions(+), 186 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 94202b5..a893f5e 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -103,7 +103,6 @@ typedef struct AcpiPmInfo {
typedef struct AcpiMiscInfo {
bool has_hpet;
bool has_tpm;
-DECLARE_BITMAP(slot_hotplug_enable, PCI_SLOT_MAX);
const unsigned char *dsdt_code;
unsigned dsdt_size;
uint16_t pvpanic_port;
@@ -647,69 +646,37 @@ static void acpi_set_pci_info(void)
}
}
-static void build_pci_bus_state_init(AcpiBuildPciBusHotplugState *state,
- AcpiBuildPciBusHotplugState *parent,
- bool pcihp_bridge_en)
+static void build_append_pcihp_notify_entry(GArray *method, int slot)
{
-state->parent = parent;
-state->device_table = build_alloc_array();
-state->notify_table = build_alloc_array();
-state->pcihp_bridge_en = pcihp_bridge_en;
-}
-
-static void build_pci_bus_state_cleanup(AcpiBuildPciBusHotplugState *state)
-{
-build_free_array(state->device_table);
-build_free_array(state->notify_table);
-}
+GArray *ifctx;
-static void *build_pci_bus_begin(PCIBus *bus, void *parent_state)
-{
-AcpiBuildPciBusHotplugState *parent = parent_state;
-AcpiBuildPciBusHotplugState *child = g_malloc(sizeof *child);
-
-build_pci_bus_state_init(child, parent, parent->pcihp_bridge_en);
+ifctx = build_alloc_array();
+build_append_byte(ifctx, 0x7B); /* AndOp */
+build_append_byte(ifctx, 0x68); /* Arg0Op */
+build_append_int(ifctx, 0x1U << slot);
+build_append_byte(ifctx, 0x00); /* NullName */
+build_append_byte(ifctx, 0x86); /* NotifyOp */
+build_append_namestring(ifctx, "S%.02X", PCI_DEVFN(slot, 0));
+build_append_byte(ifctx, 0x69); /* Arg1Op */
-return child;
+/* Pack it up */
+build_package(ifctx, 0xA0 /* IfOp */);
+build_append_array(method, ifctx);
+build_free_array(ifctx);
}
-static void build_pci_bus_end(PCIBus *bus, void *bus_state)
+static void build_append_pci_bus_devices(GArray *parent_scope, PCIBus *bus,
+ bool pcihp_bridge_en)
{
-AcpiBuildPciBusHotplugState *child = bus_state;
-AcpiBuildPciBusHotplugState *parent = child->parent;
GArray *bus_table = build_alloc_array();
-DECLARE_BITMAP(slot_hotplug_enable, PCI_SLOT_MAX);
-DECLARE_BITMAP(slot_device_present, PCI_SLOT_MAX);
-DECLARE_BITMAP(slot_device_system, PCI_SLOT_MAX);
-DECLARE_BITMAP(slot_device_vga, PCI_SLOT_MAX);
-DECLARE_BITMAP(slot_device_qxl, PCI_SLOT_MAX);
-uint8_t op;
-int i;
+GArray *method = NULL;
QObject *bsel;
-GArray *method;
-bool bus_hotplug_support = false;
-
-/*
- * Skip bridge subtree creation if bridge hotplug is disabled
- * to make acpi tables compatible with legacy machine types.
- */
-if (!child->pcihp_bridge_en && bus->parent_dev) {
-return;
-}
+PCIBus *sec;
+int i;
if (bus->parent_dev) {
-op = 0x82; /* DeviceOp */
-build_append_namestring(bus_table, "S%.02X",
- bus->parent_dev->devfn);
-build_append_byte(bus_table, 0x08); /* NameOp */
-build_append_namestring(bus_table, "_SUN");
-build_append_value(bus_table, PCI_SLOT(bus->parent_dev->devfn), 1);
-build_append_byte(bus_table, 0x08); /* NameOp */
-build_append_namestring(bus_table, "_ADR");
-build_append_value(bus_table, (PCI_SLOT(bus->parent_dev->devfn) << 16)
|
- PCI_FUNC(bus->parent_dev->devfn), 4);
+build_append_namestring(bus_table, "S%.02X_", bus->parent_dev->devfn);
} else {
-op = 0x10; /* ScopeOp */;
build_append_namestring(bus_table, "PCI0");
}
@@ -718,171 +685,103 @@ static void build_pci_bus_end(PCIBus *bus, void
*bus_state)
build_append_byte(bus_table, 0x08); /* NameOp */
build_append_namestring(bus_table, "BSEL");
build_append_int(bus_table, qint_get_int(qobject_to_qint(bsel)));
-memset(slot_hotplug_enable, 0xff, sizeof slot_hotplug_enable);
-} else {
-/* No bsel - no slots are hot-pluggable */
-memset(slot_hotplug_enable, 0x00, sizeof slot_hotplug_enable);
[Qemu-devel] [PATCH 2/2] exec: change default exception_index value for migration to -1
In QEMU 2.2 the exception_index value was added to the migration stream
through a subsection. The default was set to 0, which is wrong and
should have been -1.
However, 2.2 does not have commit e511b4d (cpu-exec: reset exception_index
correctly, 2014-11-26), hence in 2.2 the exception_index is never used
and is set to -1 on the next call to cpu_exec. So we can change the
migration stream to make the default -1. The effects are:
- 2.2.1 -> 2.2.0: cpu->exception_index set incorrectly to 0 if it
were -1 on the source; then reset to -1 in cpu_exec. This is TCG
only; KVM does not use exception_index.
- 2.2.0 -> 2.2.1: cpu->exception_index set incorrectly to -1 if it
were 0 on the source; but it would be reset to -1 in cpu_exec anyway.
This is TCG only; KVM does not use exception_index.
- 2.2.1 -> 2.1: two bugs fixed: 1) can migrate backwards if
cpu->exception_index is set to -1; 2) should not migrate backwards
(but 2.2.0 allows it) if cpu->exception_index is set to 0
- 2.2.0 -> 2.3.0: 2.2.0 will send the subsection unnecessarily if
exception_index is -1, but that is not a problem. 2.3.0 will set
cpu->exception_index to -1 if it is 0 on the source, but this would
be anyway a problem for 2.2.0 -> 2.2.x migration (due to lack of
commit e511b4d in 2.2.x) so we can ignore it
- 2.2.1 -> 2.3.0: everything works.
In addition, play it safe and never send the subsection unless TCG
is in use. KVM does not use exception_index (PPC KVM stores values
in it for use in the subsequent call to ppc_cpu_do_interrupt, but
does not need it as soon as kvm_handle_debug returns). Xen and
qtest do not run any code for the CPU at all.
Reported-by: Igor Mammedov
Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini
---
exec.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/exec.c b/exec.c
index 963481a..c2ed10a 100644
--- a/exec.c
+++ b/exec.c
@@ -434,7 +434,7 @@ static int cpu_common_pre_load(void *opaque)
{
CPUState *cpu = opaque;
-cpu->exception_index = 0;
+cpu->exception_index = -1;
return 0;
}
@@ -443,7 +443,7 @@ static bool cpu_common_exception_index_needed(void *opaque)
{
CPUState *cpu = opaque;
-return cpu->exception_index != 0;
+return tcg_enabled() && cpu->exception_index != -1;
}
static const VMStateDescription vmstate_cpu_common_exception_index = {
--
1.8.3.1
[Qemu-devel] [PATCH 0/2] cpu->exception_index fixes
Two fixes for cpu->exception_index: - breakage of linux-user from commit e511b4d - wrong subsection definition in 2.2 (see patch 2) Paolo Paolo Bonzini (2): cpu: initialize cpu->exception_index on reset exec: change default exception_index value for migration to -1 cpus.c| 3 --- exec.c| 4 ++-- qom/cpu.c | 1 + 3 files changed, 3 insertions(+), 5 deletions(-) -- 1.8.3.1
[Qemu-devel] [PATCH 1/2] cpu: initialize cpu->exception_index on reset
This unbreaks linux-user (broken by e511b4d, cpu-exec: reset exception_index
correctly, 2014-11-26).
Reported-by: Eduardo Habkost
Signed-off-by: Paolo Bonzini
---
cpus.c| 3 ---
qom/cpu.c | 1 +
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/cpus.c b/cpus.c
index 1b5168a..2edb5cd 100644
--- a/cpus.c
+++ b/cpus.c
@@ -940,7 +940,6 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
qemu_mutex_lock(&qemu_global_mutex);
qemu_thread_get_self(cpu->thread);
cpu->thread_id = qemu_get_thread_id();
-cpu->exception_index = -1;
cpu->can_do_io = 1;
current_cpu = cpu;
@@ -982,7 +981,6 @@ static void *qemu_dummy_cpu_thread_fn(void *arg)
qemu_mutex_lock_iothread();
qemu_thread_get_self(cpu->thread);
cpu->thread_id = qemu_get_thread_id();
-cpu->exception_index = -1;
cpu->can_do_io = 1;
sigemptyset(&waitset);
@@ -1026,7 +1024,6 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
CPU_FOREACH(cpu) {
cpu->thread_id = qemu_get_thread_id();
cpu->created = true;
-cpu->exception_index = -1;
cpu->can_do_io = 1;
}
qemu_cond_signal(&qemu_cpu_cond);
diff --git a/qom/cpu.c b/qom/cpu.c
index 79d2228..9c68fa4 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -249,6 +249,7 @@ static void cpu_common_reset(CPUState *cpu)
cpu->icount_extra = 0;
cpu->icount_decr.u32 = 0;
cpu->can_do_io = 0;
+cpu->exception_index = -1;
memset(cpu->tb_jmp_cache, 0, TB_JMP_CACHE_SIZE * sizeof(void *));
}
--
1.8.3.1
Re: [Qemu-devel] broken backward migration due to commit 461a275 "parallel: adding vmstate for save/restore"
On 19/12/2014 12:32, Igor Mammedov wrote: > There is one more commit that breaks it, this time with subsection > 6c3bff0 "exec: Save CPUState::exception_index field" > > qemu: warning: error while loading state for instance 0x0 of device > 'cpu_common' > the same reproducer with -parallel none Patch sent, thanks. Paolo
Re: [Qemu-devel] Review of ways to create BDSes (was: Review of monitor commands identifying BDS / BB by name)
Am 18.12.2014 um 16:25 hat Markus Armbruster geschrieben:
> = Introduction =
>
> BDSes can be opened in various ways. Some of them don't provide for
> user configuration. Some of them should.
>
> Example: -drive format=qcow2,file=foo.qcow2 where foo.qcow2 has a raw
> backing file foo.raw. This creates the the following tree of BDSes:
>
> (qcow2,foo.qcow2)
> /\
> (file,foo.qcow2)(raw,foo.raw)
> |
>(file,foo.raw)
>
> Before Kevin added driver-specific options, -drive let you configure
> basically just the root. Configuration for the others was inferred from
> the root's configuration and the images. Driver-specific options let
> you configure all the nodes. Defaults are still inferred as before.
>
> Example: blockdev-snapshot-sync provides only a small subset of the full
> configuration options for the BDS it creates. Could be fixed by
> duplicating the full options, i.e. blockdev-add's. But a command that
> just snapshots and leaves BDS creation to blockdev-add would be cleaner.
>
> This is a systematic review of all the ways you can open BDSes in qemu
> proper, i.e. not in qemu-{img,io,nbd}. I tracked them down by following
> the call chains leading to bdrv_open().
Possibly also interesting for a followup: All the ways you can reopen
BDSes with different options/flags.
> = QMP Commands =
>
> * block-commit
>
> I figure this clones the @base BDS initially, and replaces it by the
> clone finally. Is support for user configuration for this clone
> wanted?
I don't think such a clone exists. Data is directly committed to @base.
The command looks to me as if it could be okay.
> * blockdev-add
>
> Boils down to a bdrv_open(), which is discussed in the next section.
>
> * blockdev-snapshot-sync
>
> Creates a new image and a new BDS with only a few configuration
> options: @snapshot-file (the filename), @snapshot-node-name,
> @format=qcow2, @mode. Conflates three operations: create image, open
> image, snapshot. I guess we want to replace it by a basic snapshot
> operation that can be used with with blockdev-add and some command to
> create images.
Yes. We should have called this one drive-snapshot, it fits better in
the drive-* family of commands. We can call the real blockdev-style
command blockdev-snapshot - it is still synchronous technically, but it
doesn't do anything like bdrv_open() that could be blocking.
> * change
>
> Closes, then opens a BDS with just two configuration options: @target
> (the filename) and @arg (the format). Needs replacing.
Max (added to CC) is working on it.
> * drive-backup
>
> Similar to blockdev-snapshot-sync, except the filename is called
> @target, and the node name can't be configured. I guess we want to
> replace it by a basic backup operation.
>
> * drive-mirror
>
> Similar to blockdev-snapshot-sync, except the filename is called
> @target, and the node name @node-name. I guess we want to replace it
> by a basic mirror operation.
Yes. We called these drive-* instead of blockdev-* intentionally, so
that the latter names would be free for operations working on existing
BDSes.
> * transaction
>
> This is a wrapper around a list of transaction-capable commands.
> Thus, nothing new here.
>
>
> = Generic block layer =
>
> bdrv_open() opens a BDS and possibly children "file" and "backing"
> according to its configuration.
>
> Subtypes of BlockdevOptionsGenericFormat have configuration for "file".
>
> Subtypes of BlockdevOptionsGenericCOWFormat additionally have
> configuration for "backing" (defaults to "infer from COW image").
>
> bdrv_open() can additionally splice in a QCOW2 BDS to implement
> snapshot=on. No way to configure, but that's okay, because it's a
> convenience feature, and to configure you simply do the splicing
> explicitly.
>
>
> = Block driver methods =
>
> == bdrv_create() ==
>
> The BDSes created here are all internal temporaries of the method, hence
> user configuration isn't needed. Correct?
Filenames ought to be enough for everyone. Not.
But at the moment all the callers can't deal with non-filename
specifications of the image location, so that's a larger problem.
> == bdrv_file_open() ==
>
> * quorum_open()
>
> Creates / connects to its children according to configuration in
> BlockdevOptionsQuorum.
>
> * blkdebug_open()
>
> Creates / connects to its children according to configuration in
> BlockdevOptionsBlkdebug.
>
> * blkverify_open()
>
> Creates / connects to its children according to configuration in
> BlockdevOptionsBlkverify.
>
> * vvfat's enable_write_target()
>
> You don't want to know.
This would have been an interesting one for a change. ;-)
> == bdrv_open() ==
>
> * vmdk_open()
>
> Creates BDSes for its extents, configuration inferred from images.
> Looks like this needs work.
Very much so.
> = Xen =
>
> blk_connect() calls b
Re: [Qemu-devel] [PATCH 0/2] cpu->exception_index fixes
Hello, On Fri, Dec 19, 2014 at 12:53 PM, Paolo Bonzini wrote: > Two fixes for cpu->exception_index: > > - breakage of linux-user from commit e511b4d Tested-by: Laurent Desnogues Thanks, Laurent > - wrong subsection definition in 2.2 (see patch 2) > > Paolo > > Paolo Bonzini (2): > cpu: initialize cpu->exception_index on reset > exec: change default exception_index value for migration to -1 > > cpus.c| 3 --- > exec.c| 4 ++-- > qom/cpu.c | 1 + > 3 files changed, 3 insertions(+), 5 deletions(-) > > -- > 1.8.3.1 > >
Re: [Qemu-devel] [Qemu-ppc] Unable to loadvm on qemu-system-ppc -M g3beige (keyboard freeze)
On 19 December 2014 at 10:53, Mark Cave-Ayland wrote: > It seems that I've misunderstood something mentioned earlier in the > thread, i.e. that loadvm also does the create, initialize, realize and > reset cycle. At least issuing "loadvm" in the monitor doesn't seem to do > that as far as I can see here? For monitor "loadvm", we've already done create/initialize/realize as part of our initial setup of the VM when we started QEMU. reset is done by load_vmstate() calling qemu_system_reset(). For command line "-loadvm", we do the load_vmstate() after the initial reset in vl.c. Either way, VM reloading doesn't need to deal with anything that's set up in device init/realize as a fixed config, and it doesn't need to assert IRQ lines either. thanks -- PMM
Re: [Qemu-devel] broken backward migration due to commit 461a275 "parallel: adding vmstate for save/restore"
On Fri, 19 Dec 2014 13:02:24 +0100 Paolo Bonzini wrote: > > > On 19/12/2014 12:32, Igor Mammedov wrote: > > There is one more commit that breaks it, this time with subsection > > 6c3bff0 "exec: Save CPUState::exception_index field" > > > > qemu: warning: error while loading state for instance 0x0 of device > > 'cpu_common' > > the same reproducer with -parallel none > > Patch sent, thanks. > > Paolo one more breakage: a28fe7e pckbd: adding new fields to vmstate source: qemu-system-x86_64 -monitor stdio -M pc-i440fx-1.7 -parallel none xpsp3x86.qcow2 switch to text screen where you could select Safe Mode and stop/migrate at this point target: qemu-system-x86_64-1.7 -monitor stdio -M pc-i440fx-1.7 -incoming "exec: gzip -c -d STATEFILE.gz" xpsp3x86.qcow2 qemu: warning: error while loading state for instance 0x0 of device 'pckbd'
Re: [Qemu-devel] broken backward migration due to commit 461a275 "parallel: adding vmstate for save/restore"
On Fri, 19 Dec 2014 13:42:37 +0100 Igor Mammedov wrote: > On Fri, 19 Dec 2014 13:02:24 +0100 > Paolo Bonzini wrote: > > > > > > > On 19/12/2014 12:32, Igor Mammedov wrote: > > > There is one more commit that breaks it, this time with subsection > > > 6c3bff0 "exec: Save CPUState::exception_index field" > > > > > > qemu: warning: error while loading state for instance 0x0 of device > > > 'cpu_common' > > > the same reproducer with -parallel none > > > > Patch sent, thanks. > > > > Paolo > > one more breakage: > a28fe7e pckbd: adding new fields to vmstate > > source: > qemu-system-x86_64 -monitor stdio -M pc-i440fx-1.7 -parallel none > xpsp3x86.qcow2 > > switch to text screen where you could select Safe Mode and stop/migrate at > this point one more issue if you continue normal boot and wait till XP is booted and then migrate the target will error out with: qemu: warning: error while loading state for instance 0x0 of device 'serial' > > target: > qemu-system-x86_64-1.7 -monitor stdio -M pc-i440fx-1.7 -incoming "exec: gzip > -c -d STATEFILE.gz" xpsp3x86.qcow2 > > qemu: warning: error while loading state for instance 0x0 of device 'pckbd' >
Re: [Qemu-devel] [PULL 00/10] vnc: add support for multiple vnc displays
Gerd Hoffmann writes: > Hi, > > Ok, here we go with the multiple vnc display patch series. > Compared to the v3 series two patch hunks have been moved > from patch #9 to patch #2. The final result after applying > all patches hasn't changed, so I think it's ok to go straight > for a pull req instead of sending a v4. Hold your horses, I have review comments :)
Re: [Qemu-devel] [PULL 0/4] vga: cirrus hwcursor fixes.
On 17 December 2014 at 14:42, Gerd Hoffmann wrote: > Hi, > > vga patches lingering way too long in my queue. > > Finally merging the cirrus hwcursor fixes (for NT4 guests!) > discussed months ago ... > > Also minor tweaks picked up during freeze. > > please pull, > Gerd > > The following changes since commit dfa9c2a0f4d0a0c8b2c1449ecdbb1297427e1560: > > Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into > staging (2014-12-15 16:43:42 +) > > are available in the git repository at: > > > git://git.kraxel.org/qemu tags/pull-vga-20141216-1 > > for you to fetch changes up to 46817e86fc1d97af0a7d9e4060730f7b00183083: > > vga: set catagory bit for secondary vga device (2014-12-16 15:14:42 +0100) > > > cirrus hwcursor fixes. > set secondary-vga category. > > Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH 4/8] target-i386: Keep track of apic-id setting
On Fri, Dec 19, 2014 at 12:23:07PM +0100, Paolo Bonzini wrote:
> On 19/12/2014 03:41, Eduardo Habkost wrote:
> > Set a flag indicating that the apic-id property was set, so we can later
> > ensure we won't try to realize a X86CPU object before apic-id was set.
> >
> > Signed-off-by: Eduardo Habkost
> > Cc: Gu Zheng
> > ---
> > target-i386/cpu-qom.h | 1 +
> > target-i386/cpu.c | 3 ++-
> > 2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
> > index 4a6f48a..ba0ee15 100644
> > --- a/target-i386/cpu-qom.h
> > +++ b/target-i386/cpu-qom.h
> > @@ -94,6 +94,7 @@ typedef struct X86CPU {
> > bool migratable;
> > bool host_features;
> > uint32_t apic_id;
> > +bool apic_id_set;
> >
> > /* if true the CPUID code directly forward host cache leaves to the
> > guest */
> > bool cache_info_passthrough;
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index cbed717..bb9525d 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -1691,7 +1691,7 @@ static void x86_cpuid_get_apic_id(Object *obj,
> > Visitor *v, void *opaque,
> >const char *name, Error **errp)
> > {
> > X86CPU *cpu = X86_CPU(obj);
> > -int64_t value = cpu->apic_id;
> > +int64_t value = cpu->apic_id_set ? cpu->apic_id : -1;
>
> Should this return an error if the apic_id was not set?
Both approaches look valid to me. But I wouldn't expect a property to
return errors by default when read, I expect it to simply have a default
value (in this case, -1).
If we return errors, a tool/command that reads all properties from
objects would need to treat it as a special case, instead of simply
printing "-1".
Do we have any other cases where we return errors from a property getter
(especially in the default case)? I didn't find any.
--
Eduardo
Re: [Qemu-devel] [PATCH 7/8] target-i386: Move topology.h to hw/i386/topology.h
On Fri, Dec 19, 2014 at 12:24:05PM +0100, Paolo Bonzini wrote: > > > On 19/12/2014 03:41, Eduardo Habkost wrote: > > This will allow the PC code to use the header, and lets us eliminate the > > QEMU_INCLUDES hack inside tests/Makefile. > > > > Signed-off-by: Eduardo Habkost > > Please use include/hw/i386/topology.h (the toplevel build and source > directories should not be part of the #include path, so it's a bug that > this works). Oops! That was what I intended to do, but moved it to the wrong directory. I will fix it in the next version. -- Eduardo
Re: [Qemu-devel] [PULL 09/10] monitor: add query-vnc2 command
Gerd Hoffmann writes:
> Add new query vnc qmp command, for the lack of better ideas just name it
> "query-vnc2". Changes over query-vnc:
>
> * It returns a list of vnc servers, so multiple vnc server instances
>are covered.
> * Each vnc server returns a list of server sockets. Followup patch
>will use that to also report websockets. In case we add support for
>multiple server sockets server sockets (to better support ipv4+ipv6
>dualstack) we can add them to the list too.
I guess we could shoehorn this into query-vnc by having it return an
anonymous union and use an optional parameter to select the alternative.
Too much trouble just to avoid an ugly name.
Call it query-vnc-servers?
> Signed-off-by: Gerd Hoffmann
> ---
> qapi-schema.json | 68
> qmp-commands.hx | 5 +++
> ui/vnc.c | 133
> +++
> 3 files changed, 206 insertions(+)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 563b4ad..2d45d4c 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -751,6 +751,63 @@
> '*service': 'str', '*auth': 'str', '*clients': ['VncClientInfo']}
> }
>
> ##
> +# @VncPriAuth:
I don't personally mind abbreviations, but maybe somebody else does:
QAPI / QMP tends to spell things out, like VncPrimaryAuthentication.
Maybe just VncPrimaryAuth, since there's precedence for abbreviating
authentication that way.
> +#
> +# vnc primary authentication method.
> +#
> +# Since: 2.3
> +##
> +{ 'enum': 'VncPriAuth',
> + 'data': [ 'none', 'vnc', 'ra2', 'ra2ne', 'tight', 'ultra',
> +'tls', 'vencrypt', 'sasl' ] }
> +
> +##
> +# @VncVencryptSubAuth:
> +#
> +# vnc sub authentication method with vencrypt.
> +#
> +# Since: 2.3
> +##
> +{ 'enum': 'VncVencryptSubAuth',
> + 'data': [ 'plain',
> +'tls-none', 'x509-none',
> +'tls-vnc', 'x509-vnc',
> +'tls-plain', 'x509-plain',
> +'tls-sasl', 'x509-sasl' ] }
> +
> +##
> +# @VncInfo2:
> +#
> +# Information about a vnc server
> +#
> +# @id: vnc server name.
> +#
> +# @server: A list of @VncBasincInfo describing all listening sockets.
> +# The list can be empty (in case the vnc server is disabled).
> +# It also may have multiple entries: normal + websocket,
> +# possibly also ipv4 + ipv6 in the future.
> +#
> +# @clients: A list of @VncClientInfo of all currently connected clients.
> +# The list can be empty, for obvious reasons.
> +#
> +# @auth: The current authentication type used by the server
> +#
> +# @vencrypt: #optional The vencrypt sub authentication type used by the
> server,
> +#only specified in case auth == vencrypt.
> +#
> +# @display: #optional The display device the vnc server is linked to.
> +#
> +# Since: 2.3
> +##
> +{ 'type': 'VncInfo2',
> + 'data': { 'id': 'str',
> +'server': ['VncBasicInfo'],
> +'clients' : ['VncClientInfo'],
> +'auth' : 'VncPriAuth',
> +'*vencrypt' : 'VncVencryptSubAuth',
> +'*display' : 'str' } }
> +
> +##
> # @query-vnc:
> #
> # Returns information about the current VNC server
> @@ -762,6 +819,17 @@
> { 'command': 'query-vnc', 'returns': 'VncInfo' }
>
> ##
> +# @query-vnc2:
> +#
> +# Returns a list of vnc servers. The list can be empty.
> +#
> +# Returns: @VncInfo
Returns: a list of @VncInfo2
> +#
> +# Since: 2.3
> +##
> +{ 'command': 'query-vnc2', 'returns': ['VncInfo2'] }
> +
> +##
> # @SpiceBasicInfo
> #
> # The basic information for SPICE network connection
Schema looks good to me otherwise, but I'd like to hear Eric's opinion.
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 3348782..dec288a 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -2825,6 +2825,11 @@ EQMP
> .args_type = "",
> .mhandler.cmd_new = qmp_marshal_input_query_vnc,
> },
> +{
> +.name = "query-vnc2",
> +.args_type = "",
> +.mhandler.cmd_new = qmp_marshal_input_query_vnc2,
> +},
>
> SQMP
> query-spice
> diff --git a/ui/vnc.c b/ui/vnc.c
> index d7c7865..e730059 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -456,6 +456,139 @@ out_error:
> return NULL;
> }
>
> +static VncBasicInfoList *qmp_query_server_entry(int socket,
> +VncBasicInfoList *prev)
> +{
> +VncBasicInfoList *list;
> +VncBasicInfo *info;
> +struct sockaddr_storage sa;
> +socklen_t salen = sizeof(sa);
> +char host[NI_MAXHOST];
> +char serv[NI_MAXSERV];
> +
> +if (getsockname(socket, (struct sockaddr *)&sa, &salen) < 0 ||
> +getnameinfo((struct sockaddr *)&sa, salen,
> +host, sizeof(host), serv, sizeof(serv),
> +NI_NUMERICHOST | NI_NUMERICSERV) < 0) {
> +return prev;
> +}
> +
> +info = g_new0(VncBasicInfo, 1);
> +info->host = g_strdup(hos
Re: [Qemu-devel] [PULL 10/10] monitor: add vnc websockets
Copying Eric again, even though the QAPI schema change here is pretty
trivial.
Gerd Hoffmann writes:
> Add websockets bool to VncBasicInfo, report websocket server sockets,
> flag websocket client connections.
>
> Signed-off-by: Gerd Hoffmann
> ---
> qapi-schema.json | 5 -
> ui/vnc.c | 15 ---
> 2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 2d45d4c..07deb71 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -672,12 +672,15 @@
> #
> # @family: address family
> #
> +# @websocket: true in case the socket is a websocket (since 2.3).
> +#
> # Since: 2.1
> ##
> { 'type': 'VncBasicInfo',
>'data': { 'host': 'str',
> 'service': 'str',
> -'family': 'NetworkAddressFamily' } }
> +'family': 'NetworkAddressFamily',
> +'websocket': 'bool' } }
>
> ##
> # @VncServerInfo
> diff --git a/ui/vnc.c b/ui/vnc.c
> index e730059..fb8068f 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -353,6 +353,9 @@ static VncClientInfo *qmp_query_vnc_client(const VncState
> *client)
> info->base->host = g_strdup(host);
> info->base->service = g_strdup(serv);
> info->base->family = inet_netfamily(sa.ss_family);
> +#ifdef CONFIG_VNC_WS
> +info->base->websocket = client->websocket;
> +#endif
>
> #ifdef CONFIG_VNC_TLS
> if (client->tls.session && client->tls.dname) {
> @@ -457,6 +460,7 @@ out_error:
> }
>
> static VncBasicInfoList *qmp_query_server_entry(int socket,
> +bool websocket,
> VncBasicInfoList *prev)
> {
> VncBasicInfoList *list;
> @@ -477,6 +481,7 @@ static VncBasicInfoList *qmp_query_server_entry(int
> socket,
> info->host = g_strdup(host);
> info->service = g_strdup(serv);
> info->family = inet_netfamily(sa.ss_family);
> +info->websocket = websocket;
>
> list = g_new0(VncBasicInfoList, 1);
> list->value = info;
> @@ -572,12 +577,13 @@ VncInfo2List *qmp_query_vnc2(Error **errp)
> info->display = g_strdup(dev->id);
> }
> if (vd->lsock != -1) {
> -info->server = qmp_query_server_entry(vd->lsock,
> +info->server = qmp_query_server_entry(vd->lsock, false,
>info->server);
> }
> #ifdef CONFIG_VNC_WS
> if (vd->lwebsock != -1) {
> -/* TODO */
> +info->server = qmp_query_server_entry(vd->lwebsock, true,
> + info->server);
> }
> #endif
>
> @@ -3304,10 +3310,13 @@ void vnc_display_open(const char *id, Error **errp)
> {
> VncDisplay *vs = vnc_display_find(id);
> QemuOpts *opts = qemu_opts_find(&qemu_vnc_opts, id);
> -const char *display, *websocket, *share, *device_id;
> +const char *display, *share, *device_id;
> QemuConsole *con;
> int password = 0;
> int reverse = 0;
> +#ifdef CONFIG_VNC_WS
> +const char *websocket;
> +#endif
> #ifdef CONFIG_VNC_TLS
> int tls = 0, x509 = 0;
> const char *path;
Reviewed-by: Markus Armbruster
Re: [Qemu-devel] broken backward migration due to commit 461a275 "parallel: adding vmstate for save/restore"
On 19/12/2014 13:42, Igor Mammedov wrote:
> On Fri, 19 Dec 2014 13:02:24 +0100
> Paolo Bonzini wrote:
>
>>
>>
>> On 19/12/2014 12:32, Igor Mammedov wrote:
>>> There is one more commit that breaks it, this time with subsection
>>> 6c3bff0 "exec: Save CPUState::exception_index field"
>>>
>>> qemu: warning: error while loading state for instance 0x0 of device
>>> 'cpu_common'
>>> the same reproducer with -parallel none
>>
>> Patch sent, thanks.
>>
>> Paolo
>
> one more breakage:
> a28fe7e pckbd: adding new fields to vmstate
>
> source:
> qemu-system-x86_64 -monitor stdio -M pc-i440fx-1.7 -parallel none
> xpsp3x86.qcow2
>
> switch to text screen where you could select Safe Mode and stop/migrate at
> this point
>
> target:
> qemu-system-x86_64-1.7 -monitor stdio -M pc-i440fx-1.7 -incoming "exec: gzip
> -c -d STATEFILE.gz" xpsp3x86.qcow2
>
> qemu: warning: error while loading state for instance 0x0 of device 'pckbd'
Looks like Windows (all versions) writes 0xDD and 0xDF to the outport
port of the keyboard controller when it enables/disables the A20 line.
There are two possibilities:
1) disable this if you care about backwards-migration
2) apply upstream a patch like this:
diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
index 2b0cd3d..cb4a8be 100644
--- a/hw/input/pckbd.c
+++ b/hw/input/pckbd.c
@@ -373,7 +373,7 @@ static void kbd_reset(void *opaque)
static uint8_t kbd_outport_default(KBDState *s)
{
-return KBD_OUT_RESET | KBD_OUT_A20
+return (0xdf & ~KBD_OUT_OBF & ~KBD_OUT_MOUSE_OBF)
| (s->status & KBD_STAT_OBF ? KBD_OUT_OBF : 0)
| (s->status & KBD_STAT_MOUSE_OBF ? KBD_OUT_MOUSE_OBF : 0);
}
3) do both, so upstream works better
Paolo
Re: [Qemu-devel] [PATCH] net: Add persistent flag to -net tap option
On Mon, Dec 15, 2014 at 02:05:23PM +0200, Roy Vardi wrote:
> From: Roy Vardi
>
> Add 'persistent' boolean flag to -net tap option.
> When set to off - tap interface will be released on shutdown
> When set to on\not specified - tap interface will remain
>
> Running with -net tap,persistent=off will force the tap interface
> down when qemu goes down, thus ensuring that there're no zombie tap
> interfaces left
>
> This is achieved using another ioctl
>
> Note: This commit includes the above support only for linux systems
I don't understand the point of this patch. The following doesn't
persist the tun interface:
qemu-system-i386 -net tap,script=myscript.sh,downscript=no -net nic
You are changing the default to persist the interface, won't this cause
problems for existing users who don't expect persistent interfaces?
> @@ -109,6 +110,17 @@ int tap_open(char *ifname, int ifname_size, int
> *vnet_hdr,
> close(fd);
> return -1;
> }
> +
> +if (!persistent_required) {
> +ret = ioctl(fd, TUNSETPERSIST, 0);
> +if (ret != 0) {
Indentation is off here. QEMU uses 4-space indentation and this if
statement should not be indented.
> +error_report("could not configure non-persistent %s (%s):
> %m",
> + PATH_NET_TUN, ifr.ifr_name);
> +close(fd);
> +return -1;
> +}
> +}
> +
> pstrcpy(ifname, ifname_size, ifr.ifr_name);
> fcntl(fd, F_SETFL, O_NONBLOCK);
> return fd;
> diff --git a/net/tap.c b/net/tap.c
> index bde6b58..43267bb 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -556,7 +556,8 @@ int net_init_bridge(const NetClientOptions *opts, const
> char *name,
>
> static int net_tap_init(const NetdevTapOptions *tap, int *vnet_hdr,
> const char *setup_script, char *ifname,
> -size_t ifname_sz, int mq_required)
> +size_t ifname_sz, int mq_required,
> +int persistent_reuired)
Typo s/reuired/required/
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 10b9568..d26215a 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1417,7 +1417,7 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
> "-net tap[,vlan=n][,name=str],ifname=name\n"
> "connect the host TAP network interface to VLAN 'n'\n"
> #else
> -"-net
> tap[,vlan=n][,name=str][,fd=h][,fds=x:y:...:z][,ifname=name][,script=file][,downscript=dfile][,helper=helper][,sndbuf=nbytes][,vnet_hdr=on|off][,vhost=on|off][,vhostfd=h][,vhostfds=x:y:...:z][,vhostforce=on|off][,queues=n]\n"
> +"-net
> tap[,vlan=n][,name=str][,fd=h][,fds=x:y:...:z][,ifname=name][,script=file][,downscript=dfile][,helper=helper][,sndbuf=nbytes][,vnet_hdr=on|off][,vhost=on|off][,vhostfd=h][,vhostfds=x:y:...:z][,vhostforce=on|off][,queues=n][persistent=on|off]\n"
Missing comma:
[,persistent=on|off]
> "connect the host TAP network interface to VLAN 'n'\n"
> "use network scripts 'file' (default="
> DEFAULT_NETWORK_SCRIPT ")\n"
> "to configure it and 'dfile' (default="
> DEFAULT_NETWORK_DOWN_SCRIPT ")\n"
> @@ -1437,6 +1437,7 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
> "use 'vhostfd=h' to connect to an already opened vhost
> net device\n"
> "use 'vhostfds=x:y:...:z to connect to multiple already
> opened vhost net devices\n"
> "use 'queues=n' to specify the number of queues to be
> created for multiqueue TAP\n"
> +"use persistent=off to release the TAP interface on
> shutdown (default=on)\n"
Please use the same formatting as for the other options:
use 'persistent=off' to release ...
pgpFnByqYzCHa.pgp
Description: PGP signature
Re: [Qemu-devel] [PATCH 0/2] net: Trivial cleanups around g_malloc()
On Thu, Dec 04, 2014 at 02:28:15PM +0100, Markus Armbruster wrote: > Markus Armbruster (2): > net: Fuse g_malloc(); memset() into g_new0() > net: Use g_new() & friends where that makes obvious sense > > net/l2tpv3.c | 9 - > net/queue.c | 2 +- > net/slirp.c | 2 +- > 3 files changed, 6 insertions(+), 7 deletions(-) > > -- > 1.9.3 > > Thanks, applied to my net tree: https://github.com/stefanha/qemu/commits/net Stefan pgpNgnJTTwwIU.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH 0/2] tcg: Move TCG arch-specific initialization inside TCG code
On Fri, Dec 19, 2014 at 11:29:34AM +0100, Paolo Bonzini wrote: > On 19/12/2014 10:47, Andreas Färber wrote: > > How do you imagine this to work with multiple CPU types? From the looks > > of it, this is for the target CPU, not the TCG architecture, so > > tcg_arch_init() limits us to one implementation unlike now. > > On the other hand it doesn't seem to be a per-CPU, i.e. CPUClass, > > initialization either... > > I think these should go in a separate CPUClass method that is called by > tcg_exec_init (only for first_cpu now; later for all CPUs if each CPU > gets a separate tcg_ctx). configure_accelerator() (and tcg_exec_init()) is called before machine_class->init() (where the CPUs are created). I have work in progress that introduces arch-specific TYPE_ACCEL subclasses. With this, we can then make tcg_arch_init() a TYPE_TCG_ACCEL method that arch-specific TYPE_TCG_ACCEL subclasses can implement. Is it already possible to build a QEMU binary with multiple TCG arches, today? -- Eduardo
Re: [Qemu-devel] [PATCH v2] e1000: defer packets until BM enabled
On Mon, Dec 01, 2014 at 08:06:52PM +0200, Michael S. Tsirkin wrote: > Some guests seem to set BM for e1000 after > enabling RX. > If packets arrive in the window, device is wedged. > Probably works by luck on real hardware, work around > this by making can_receive depend on BM. > > Tested-by: Gabriel Somlo > Signed-off-by: Michael S. Tsirkin > --- > > Amos - you were the one reporting the failures, could > you pls confirm this patch fixes the issues for you? > > hw/net/e1000.c | 21 - > 1 file changed, 20 insertions(+), 1 deletion(-) Thanks, applied to my net tree: https://github.com/stefanha/qemu/commits/net Stefan pgpwj1JV9e0WO.pgp Description: PGP signature
[Qemu-devel] [PULL 0/5] Net patches
The following changes since commit 86b182ac0e0b44726d85598cbefb468ed22517fc: Merge remote-tracking branch 'remotes/xtensa/tags/20141217-xtensa' into staging (2014-12-17 17:31:26 +) are available in the git repository at: git://github.com/stefanha/qemu.git tags/net-pull-request for you to fetch changes up to 20302e71a5b654d7b4d0d61c7384e9dd8d112971: e1000: defer packets until BM enabled (2014-12-19 13:17:06 +) Jason Wang (1): net: don't use set/get_pointer() in set/get_netdev() Markus Armbruster (2): net: Fuse g_malloc(); memset() into g_new0() net: Use g_new() & friends where that makes obvious sense Michael S. Tsirkin (1): e1000: defer packets until BM enabled Wangkai (Kevin,C) (1): tap: fix vcpu long time io blocking on tap hw/core/qdev-properties-system.c | 82 +--- hw/net/e1000.c | 21 +- net/l2tpv3.c | 9 ++--- net/queue.c | 2 +- net/slirp.c | 2 +- net/tap.c| 12 ++ 6 files changed, 82 insertions(+), 46 deletions(-) -- 2.1.0
[Qemu-devel] [PULL 2/5] net: don't use set/get_pointer() in set/get_netdev()
From: Jason Wang
Commit 1ceef9f27359cbe92ef124bf74de6f792e71f6fb (net: multiqueue
support) tries to use set_pointer() and get_pointer() to set and get
NICPeers which is not a pointer defined in DEFINE_PROP_NETDEV. This
trick works but result a unclean and fragile implementation (e.g
print_netdev and parse_netdev).
This patch solves this issue by not using set/get_pinter() and set and
get netdev directly in set_netdev() and get_netdev(). After this the
parse_netdev() and print_netdev() were no longer used and dropped from
the source.
[Renamed 'err' label to 'out' as suggested by Markus Armbruster.
--Stefan]
Cc: Markus Armbruster
Cc: Stefan Hajnoczi
Cc: Peter Maydell
Signed-off-by: Jason Wang
Reviewed-by: Markus Armbruster
Signed-off-by: Stefan Hajnoczi
---
hw/core/qdev-properties-system.c | 82 +---
1 file changed, 44 insertions(+), 38 deletions(-)
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 65901ef..a2e44bd 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -177,42 +177,69 @@ PropertyInfo qdev_prop_chr = {
};
/* --- netdev device --- */
+static void get_netdev(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+DeviceState *dev = DEVICE(obj);
+Property *prop = opaque;
+NICPeers *peers_ptr = qdev_get_prop_ptr(dev, prop);
+char *p = g_strdup(peers_ptr->ncs[0] ? peers_ptr->ncs[0]->name : "");
-static int parse_netdev(DeviceState *dev, const char *str, void **ptr)
+visit_type_str(v, &p, name, errp);
+g_free(p);
+}
+
+static void set_netdev(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
{
-NICPeers *peers_ptr = (NICPeers *)ptr;
+DeviceState *dev = DEVICE(obj);
+Property *prop = opaque;
+NICPeers *peers_ptr = qdev_get_prop_ptr(dev, prop);
NetClientState **ncs = peers_ptr->ncs;
NetClientState *peers[MAX_QUEUE_NUM];
-int queues, i = 0;
-int ret;
+Error *local_err = NULL;
+int queues, err = 0, i = 0;
+char *str;
+
+if (dev->realized) {
+qdev_prop_set_after_realize(dev, name, errp);
+return;
+}
+
+visit_type_str(v, &str, name, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
queues = qemu_find_net_clients_except(str, peers,
NET_CLIENT_OPTIONS_KIND_NIC,
MAX_QUEUE_NUM);
if (queues == 0) {
-ret = -ENOENT;
-goto err;
+err = -ENOENT;
+goto out;
}
if (queues > MAX_QUEUE_NUM) {
-ret = -E2BIG;
-goto err;
+error_setg(errp, "queues of backend '%s'(%d) exceeds QEMU
limitation(%d)",
+ str, queues, MAX_QUEUE_NUM);
+goto out;
}
for (i = 0; i < queues; i++) {
if (peers[i] == NULL) {
-ret = -ENOENT;
-goto err;
+err = -ENOENT;
+goto out;
}
if (peers[i]->peer) {
-ret = -EEXIST;
-goto err;
+err = -EEXIST;
+goto out;
}
if (ncs[i]) {
-ret = -EINVAL;
-goto err;
+err = -EINVAL;
+goto out;
}
ncs[i] = peers[i];
@@ -221,30 +248,9 @@ static int parse_netdev(DeviceState *dev, const char *str,
void **ptr)
peers_ptr->queues = queues;
-return 0;
-
-err:
-return ret;
-}
-
-static char *print_netdev(void *ptr)
-{
-NetClientState *netdev = ptr;
-const char *val = netdev->name ? netdev->name : "";
-
-return g_strdup(val);
-}
-
-static void get_netdev(Object *obj, Visitor *v, void *opaque,
- const char *name, Error **errp)
-{
-get_pointer(obj, v, opaque, print_netdev, name, errp);
-}
-
-static void set_netdev(Object *obj, Visitor *v, void *opaque,
- const char *name, Error **errp)
-{
-set_pointer(obj, v, opaque, parse_netdev, name, errp);
+out:
+error_set_from_qdev_prop_error(errp, err, dev, prop, str);
+g_free(str);
}
PropertyInfo qdev_prop_netdev = {
--
2.1.0
[Qemu-devel] [PULL 3/5] net: Fuse g_malloc(); memset() into g_new0()
From: Markus Armbruster Signed-off-by: Markus Armbruster Signed-off-by: Stefan Hajnoczi --- net/l2tpv3.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/net/l2tpv3.c b/net/l2tpv3.c index 3b805a7..6014c43 100644 --- a/net/l2tpv3.c +++ b/net/l2tpv3.c @@ -695,8 +695,7 @@ int net_init_l2tpv3(const NetClientOptions *opts, goto outerr; } -s->dgram_dst = g_malloc(sizeof(struct sockaddr_storage)); -memset(s->dgram_dst, '\0' , sizeof(struct sockaddr_storage)); +s->dgram_dst = g_new0(struct sockaddr_storage, 1); memcpy(s->dgram_dst, result->ai_addr, result->ai_addrlen); s->dst_size = result->ai_addrlen; -- 2.1.0
Re: [Qemu-devel] [PATCH] net: Add persistent flag to -net tap option
On Fri, Dec 19, 2014 at 01:13:50PM +, Stefan Hajnoczi wrote: > On Mon, Dec 15, 2014 at 02:05:23PM +0200, Roy Vardi wrote: > > From: Roy Vardi > > > > Add 'persistent' boolean flag to -net tap option. > > When set to off - tap interface will be released on shutdown > > When set to on\not specified - tap interface will remain > > > > Running with -net tap,persistent=off will force the tap interface > > down when qemu goes down, thus ensuring that there're no zombie tap > > interfaces left > > > > This is achieved using another ioctl > > > > Note: This commit includes the above support only for linux systems > > I don't understand the point of this patch. The following doesn't > persist the tun interface: > > qemu-system-i386 -net tap,script=myscript.sh,downscript=no -net nic > > You are changing the default to persist the interface, won't this cause > problems for existing users who don't expect persistent interfaces? Yea, changing the default to persist interfaces is going to cause existing apps using this syntax to leak these tap devices on QEMU shutdown. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
[Qemu-devel] [PULL 4/5] net: Use g_new() & friends where that makes obvious sense
From: Markus Armbruster
g_new(T, n) is neater than g_malloc(sizeof(T) * n). It's also safer,
for two reasons. One, it catches multiplication overflowing size_t.
Two, it returns T * rather than void *, which lets the compiler catch
more type errors.
This commit only touches allocations with size arguments of the form
sizeof(T).
Signed-off-by: Markus Armbruster
Signed-off-by: Stefan Hajnoczi
---
net/l2tpv3.c | 6 +++---
net/queue.c | 2 +-
net/slirp.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/l2tpv3.c b/net/l2tpv3.c
index 6014c43..8c598b0 100644
--- a/net/l2tpv3.c
+++ b/net/l2tpv3.c
@@ -489,12 +489,12 @@ static struct mmsghdr *build_l2tpv3_vector(NetL2TPV3State
*s, int count)
struct iovec *iov;
struct mmsghdr *msgvec, *result;
-msgvec = g_malloc(sizeof(struct mmsghdr) * count);
+msgvec = g_new(struct mmsghdr, count);
result = msgvec;
for (i = 0; i < count ; i++) {
msgvec->msg_hdr.msg_name = NULL;
msgvec->msg_hdr.msg_namelen = 0;
-iov = g_malloc(sizeof(struct iovec) * IOVSIZE);
+iov = g_new(struct iovec, IOVSIZE);
msgvec->msg_hdr.msg_iov = iov;
iov->iov_base = g_malloc(s->header_size);
iov->iov_len = s->header_size;
@@ -729,7 +729,7 @@ int net_init_l2tpv3(const NetClientOptions *opts,
}
s->msgvec = build_l2tpv3_vector(s, MAX_L2TPV3_MSGCNT);
-s->vec = g_malloc(sizeof(struct iovec) * MAX_L2TPV3_IOVCNT);
+s->vec = g_new(struct iovec, MAX_L2TPV3_IOVCNT);
s->header_buf = g_malloc(s->header_size);
qemu_set_nonblock(fd);
diff --git a/net/queue.c b/net/queue.c
index f948318..ebbe2bb 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -62,7 +62,7 @@ NetQueue *qemu_new_net_queue(void *opaque)
{
NetQueue *queue;
-queue = g_malloc0(sizeof(NetQueue));
+queue = g_new0(NetQueue, 1);
queue->opaque = opaque;
queue->nq_maxlen = 1;
diff --git a/net/slirp.c b/net/slirp.c
index 377d7ef..0cbca3c 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -652,7 +652,7 @@ static int slirp_guestfwd(SlirpState *s, const char
*config_str,
return -1;
}
} else {
-fwd = g_malloc(sizeof(struct GuestFwd));
+fwd = g_new(struct GuestFwd, 1);
fwd->hd = qemu_chr_new(buf, p, NULL);
if (!fwd->hd) {
error_report("could not open guest forwarding device '%s'", buf);
--
2.1.0
Re: [Qemu-devel] broken backward migration due to commit 461a275 "parallel: adding vmstate for save/restore"
On 19/12/2014 13:49, Igor Mammedov wrote: >> source: >> qemu-system-x86_64 -monitor stdio -M pc-i440fx-1.7 -parallel none >> xpsp3x86.qcow2 >> >> switch to text screen where you could select Safe Mode and stop/migrate at >> this point > > one more issue if you continue normal boot and wait till XP is booted and > then migrate > the target will error out with: > > qemu: warning: error while loading state for instance 0x0 of device 'serial' I cannot reproduce this. Paolo
[Qemu-devel] [PULL 5/5] e1000: defer packets until BM enabled
From: "Michael S. Tsirkin"
Some guests seem to set BM for e1000 after
enabling RX.
If packets arrive in the window, device is wedged.
Probably works by luck on real hardware, work around
this by making can_receive depend on BM.
Tested-by: Gabriel Somlo
Signed-off-by: Michael S. Tsirkin
Reviewed-by: Jason Wang
Signed-off-by: Stefan Hajnoczi
---
hw/net/e1000.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index e33a4da..89c5788 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -33,6 +33,7 @@
#include "sysemu/sysemu.h"
#include "sysemu/dma.h"
#include "qemu/iov.h"
+#include "qemu/range.h"
#include "e1000_regs.h"
@@ -923,7 +924,9 @@ e1000_can_receive(NetClientState *nc)
E1000State *s = qemu_get_nic_opaque(nc);
return (s->mac_reg[STATUS] & E1000_STATUS_LU) &&
-(s->mac_reg[RCTL] & E1000_RCTL_EN) && e1000_has_rxbufs(s, 1);
+(s->mac_reg[RCTL] & E1000_RCTL_EN) &&
+(s->parent_obj.config[PCI_COMMAND] & PCI_COMMAND_MASTER) &&
+e1000_has_rxbufs(s, 1);
}
static uint64_t rx_desc_base(E1000State *s)
@@ -1529,6 +1532,20 @@ static NetClientInfo net_e1000_info = {
.link_status_changed = e1000_set_link_status,
};
+static void e1000_write_config(PCIDevice *pci_dev, uint32_t address,
+uint32_t val, int len)
+{
+E1000State *s = E1000(pci_dev);
+
+pci_default_write_config(pci_dev, address, val, len);
+
+if (range_covers_byte(address, len, PCI_COMMAND) &&
+(pci_dev->config[PCI_COMMAND] & PCI_COMMAND_MASTER)) {
+qemu_flush_queued_packets(qemu_get_queue(s->nic));
+}
+}
+
+
static int pci_e1000_init(PCIDevice *pci_dev)
{
DeviceState *dev = DEVICE(pci_dev);
@@ -1539,6 +1556,8 @@ static int pci_e1000_init(PCIDevice *pci_dev)
int i;
uint8_t *macaddr;
+pci_dev->config_write = e1000_write_config;
+
pci_conf = pci_dev->config;
/* TODO: RST# value should be 0, PCI spec 6.2.4 */
--
2.1.0
[Qemu-devel] [PULL 1/5] tap: fix vcpu long time io blocking on tap
From: "Wangkai (Kevin,C)"
[Adjusted doc comment for grammar.
--Stefan]
Signed-off-by: Wangkai
Signed-off-by: Stefan Hajnoczi
---
net/tap.c | 12
1 file changed, 12 insertions(+)
diff --git a/net/tap.c b/net/tap.c
index bde6b58..1fe0edf 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -189,6 +189,7 @@ static void tap_send(void *opaque)
{
TAPState *s = opaque;
int size;
+int packets = 0;
while (qemu_can_send_packet(&s->nc)) {
uint8_t *buf = s->buf;
@@ -210,6 +211,17 @@ static void tap_send(void *opaque)
} else if (size < 0) {
break;
}
+
+/*
+ * When the host keeps receiving more packets while tap_send() is
+ * running we can hog the QEMU global mutex. Limit the number of
+ * packets that are processed per tap_send() callback to prevent
+ * stalling the guest.
+ */
+packets++;
+if (packets >= 50) {
+break;
+}
}
}
--
2.1.0
[Qemu-devel] [PATCH] check return value of fcntl() to detect invalid fd
Passing some invalid fds in QEMU commandline, the fds don't exist.
QEMU will get error "TUNGETIFF ioctl() failed: Bad file descriptor",
and coredump in setting queues.
This patch checked return value of first operate to fd, QEMU will
report error and exit without coredump. It's effected for both netdev
fds and vhost_net fds.
Signed-off-by: Amos Kong
---
net/tap.c | 16 +---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/net/tap.c b/net/tap.c
index bde6b58..039280a 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -688,7 +688,7 @@ int net_init_tap(const NetClientOptions *opts, const char
*name,
NetClientState *peer)
{
const NetdevTapOptions *tap;
-int fd, vnet_hdr = 0, i = 0, queues;
+int fd, vnet_hdr = 0, i = 0, queues, ret;
/* for the no-fd, no-helper case */
const char *script = NULL; /* suppress wrong "uninit'd use" gcc warning */
const char *downscript = NULL;
@@ -722,7 +722,12 @@ int net_init_tap(const NetClientOptions *opts, const char
*name,
return -1;
}
-fcntl(fd, F_SETFL, O_NONBLOCK);
+ret = fcntl(fd, F_SETFL, O_NONBLOCK);
+if (ret < 0) {
+error_report("Fail to set file status to nonblock, "
+ "%s", strerror(-ret));
+return -1;
+}
vnet_hdr = tap_probe_vnet_hdr(fd);
@@ -761,7 +766,12 @@ int net_init_tap(const NetClientOptions *opts, const char
*name,
return -1;
}
-fcntl(fd, F_SETFL, O_NONBLOCK);
+ret = fcntl(fd, F_SETFL, O_NONBLOCK);
+if (ret < 0) {
+error_report("Fail to set file status to nonblock, "
+ "%s", strerror(-ret));
+return -1;
+}
if (i == 0) {
vnet_hdr = tap_probe_vnet_hdr(fd);
--
2.1.0
Re: [Qemu-devel] [PATCH v3 4/5] Split the QEMU buffered file code out
On Thu, Dec 18, 2014 at 09:24:11AM +, Dr. David Alan Gilbert wrote:
> * David Gibson (da...@gibson.dropbear.id.au) wrote:
> > On Fri, Dec 12, 2014 at 11:13:41AM +, Dr. David Alan Gilbert (git)
> > wrote:
> > > From: "Dr. David Alan Gilbert"
> > >
> > > The splitting of qemu-file and addition of the buffered file landed
> > > at the same time; so now split the buffered file code out.
> > >
> > > Signed-off-by: Dr. David Alan Gilbert
> > > ---
> > > migration/Makefile.objs | 2 +-
> > > migration/qemu-file-buf.c | 486
> > > ++
> > > migration/qemu-file.c | 455
> > > ---
> > > tests/Makefile| 3 +-
> > > 4 files changed, 489 insertions(+), 457 deletions(-)
> > > create mode 100644 migration/qemu-file-buf.c
> > >
> > > diff --git a/migration/Makefile.objs b/migration/Makefile.objs
> > > index ce1e3c7..d929e96 100644
> > > --- a/migration/Makefile.objs
> > > +++ b/migration/Makefile.objs
> > > @@ -1,6 +1,6 @@
> > > common-obj-y += migration.o tcp.o
> > > common-obj-y += vmstate.o
> > > -common-obj-y += qemu-file.o qemu-file-unix.o qemu-file-stdio.o
> > > +common-obj-y += qemu-file.o qemu-file-buf.o qemu-file-unix.o
> > > qemu-file-stdio.o
> > > common-obj-y += xbzrle.o
> > >
> > > common-obj-$(CONFIG_RDMA) += rdma.o
> > > diff --git a/migration/qemu-file-buf.c b/migration/qemu-file-buf.c
> > > new file mode 100644
> > > index 000..d33dd44
> > > --- /dev/null
> > > +++ b/migration/qemu-file-buf.c
> > > @@ -0,0 +1,486 @@
> > > +/*
> > > + * QEMU System Emulator
> > > + *
> > > + * Copyright (c) 2003-2008 Fabrice Bellard
> >
> > Bit hard to believe that only Fabrice listed on this file is correct,
> > given the buffered file stuff is fairly new.
>
> Yes, I'd be happy to add Stefan and Joel's name to that, although
> they never added it in their original patch, and when splitting files
> we do normally take the copyright header from what we split out of;
> but you are right it's misleading.
This is often a problem when we split source files, as the copyright
notices are rarely updated when people add new code. IANAL, but copying
the existing copyright header from the original file seems to be the
most reasonable thing to do. If people are unhappy with the existing
copyright header, they can submit patches for it.
Perhaps we could add a note just below the copyright info, indicating
that the code was originally in ${ORIGINAL_FILE}.c, to help copyright
archaeologists from the future.
--
Eduardo
Re: [Qemu-devel] [PATCH v3 6/8] acpi: add build_append_namestring() helper
On 19.12.2014 12:47, Igor Mammedov wrote:
> Use build_append_namestring() instead of build_append_nameseg()
> So user won't have to care whether name is NameSeg, NamePath or
> NameString.
>
> See for refernce ACPI 5.0: 20.2.2 Name Objects Encoding
>
> Signed-off-by: Igor Mammedov
> ---
> v2:
> assert on wrong Segcount earlier and extend condition to
> seg_count <= 0xFF || seg_count < 1
> ---
> hw/acpi/acpi_gen_utils.c | 90
> +++-
> hw/i386/acpi-build.c | 35 +++-
> include/hw/acpi/acpi_gen_utils.h | 2 +-
> 3 files changed, 106 insertions(+), 21 deletions(-)
>
> diff --git a/hw/acpi/acpi_gen_utils.c b/hw/acpi/acpi_gen_utils.c
> index 5f64c37..d5fca8e 100644
> --- a/hw/acpi/acpi_gen_utils.c
> +++ b/hw/acpi/acpi_gen_utils.c
> @@ -31,7 +31,7 @@ void build_append_array(GArray *array, GArray *val)
>
> #define ACPI_NAMESEG_LEN 4
>
> -void GCC_FMT_ATTR(2, 3)
> +static void GCC_FMT_ATTR(2, 3)
> build_append_nameseg(GArray *array, const char *format, ...)
> {
> /* It would be nicer to use g_string_vprintf but it's only there in 2.22
> */
> @@ -50,6 +50,94 @@ build_append_nameseg(GArray *array, const char *format,
> ...)
> g_array_append_vals(array, "", ACPI_NAMESEG_LEN - len);
> }
>
> +static const uint8_t ACPI_DualNamePrefix = 0x2E;
> +static const uint8_t ACPI_MultiNamePrefix = 0x2F;
> +
> +static void
> +build_append_namestringv(GArray *array, const char *format, va_list ap)
> +{
> +/* It would be nicer to use g_string_vprintf but it's only there in 2.22
> */
> +char *s;
> +int len;
> +va_list va_len;
> +char **segs;
> +char **segs_iter;
> +int seg_count = 0;
> +
> +va_copy(va_len, ap);
> +len = vsnprintf(NULL, 0, format, va_len);
> +va_end(va_len);
> +len += 1;
> +s = g_new(typeof(*s), len);
> +
> +len = vsnprintf(s, len, format, ap);
> +
> +segs = g_strsplit(s, ".", 0);
> +g_free(s);
> +
> +/* count segments */
> +segs_iter = segs;
> +while (*segs_iter) {
> +++segs_iter;
> +++seg_count;
> +}
> +/*
> + * ACPI 5.0 spec: 20.2.2 Name Objects Encoding:
> + * "SegCount can be from 1 to 255"
> + */
> +assert(seg_count <= 0xFF || seg_count < 1);
hmm that doesn't seem right. If seg_count is 0, you want to fail.
assert(seg_count <= 0xff && seg_count != 0);
or
assert(seg_count > 0 && seg_count <= 255);
or something like that..
> +
> +/* handle RootPath || PrefixPath */
> +s = *segs;
> +while (*s == '\\' || *s == '^') {
> +g_array_append_val(array, *s);
> +++s;
> +}
> +
> +switch (seg_count) {
> +case 1:
> +if (!*s) { /* NullName */
> +const uint8_t nullpath = 0;
> +g_array_append_val(array, nullpath);
> +} else {
> +build_append_nameseg(array, "%s", s);
> +}
> +break;
> +
> +case 2:
> +g_array_append_val(array, ACPI_DualNamePrefix);
> +build_append_nameseg(array, "%s", s);
> +build_append_nameseg(array, "%s", segs[1]);
> +break;
> +
> +default:
> +g_array_append_val(array, ACPI_MultiNamePrefix);
> +g_array_append_val(array, seg_count);
> +
> +/* handle the 1st segment manually due to prefix/root path */
> +build_append_nameseg(array, "%s", s);
> +
> +/* add the rest of segments */
> +segs_iter = segs + 1;
> +while (*segs_iter) {
> +build_append_nameseg(array, "%s", *segs_iter);
> +++segs_iter;
> +}
> +break;
> +}
> +g_strfreev(segs);
> +}
> +
> +void GCC_FMT_ATTR(2, 3)
> +build_append_namestring(GArray *array, const char *format, ...)
> +{
> +va_list ap;
> +
> +va_start(ap, format);
> +build_append_namestringv(array, format, ap);
> +va_end(ap);
> +}
> +
> /* 5.4 Definition Block Encoding */
> enum {
> PACKAGE_LENGTH_1BYTE_SHIFT = 6, /* Up to 63 - use extra 2 bits. */
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index f7282ef..7642f6d 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -279,7 +279,7 @@ static GArray *build_alloc_method(const char *name,
> uint8_t arg_count)
> {
> GArray *method = build_alloc_array();
>
> -build_append_nameseg(method, "%s", name);
> +build_append_namestring(method, "%s", name);
> build_append_byte(method, arg_count); /* MethodFlags: ArgCount */
>
> return method;
> @@ -571,7 +571,7 @@ build_append_notify_method(GArray *device, const char
> *name,
>
> for (i = 0; i < count; i++) {
> GArray *target = build_alloc_array();
> -build_append_nameseg(target, format, i);
> +build_append_namestring(target, format, i);
> assert(i < 256); /* Fits in 1 byte */
> build_append_notify_target_ifequal(method, target, i, 1);
> build_free_array(target);
> @@ -699,24 +699,24 @@ static void build_pci
Re: [Qemu-devel] [PATCH 1/2] cpu: initialize cpu->exception_index on reset
On Fri, Dec 19, 2014 at 12:53:13PM +0100, Paolo Bonzini wrote: > This unbreaks linux-user (broken by e511b4d, cpu-exec: reset exception_index > correctly, 2014-11-26). > > Reported-by: Eduardo Habkost > Signed-off-by: Paolo Bonzini Fixes the problem. Thanks! Tested-by: Eduardo Habkost -- Eduardo
Re: [Qemu-devel] Review of ways to create BDSes
Kevin Wolf writes:
> Am 18.12.2014 um 16:25 hat Markus Armbruster geschrieben:
>> = Introduction =
>>
>> BDSes can be opened in various ways. Some of them don't provide for
>> user configuration. Some of them should.
>>
>> Example: -drive format=qcow2,file=foo.qcow2 where foo.qcow2 has a raw
>> backing file foo.raw. This creates the the following tree of BDSes:
>>
>> (qcow2,foo.qcow2)
>> /\
>> (file,foo.qcow2)(raw,foo.raw)
>> |
>>(file,foo.raw)
>>
>> Before Kevin added driver-specific options, -drive let you configure
>> basically just the root. Configuration for the others was inferred from
>> the root's configuration and the images. Driver-specific options let
>> you configure all the nodes. Defaults are still inferred as before.
>>
>> Example: blockdev-snapshot-sync provides only a small subset of the full
>> configuration options for the BDS it creates. Could be fixed by
>> duplicating the full options, i.e. blockdev-add's. But a command that
>> just snapshots and leaves BDS creation to blockdev-add would be cleaner.
>>
>> This is a systematic review of all the ways you can open BDSes in qemu
>> proper, i.e. not in qemu-{img,io,nbd}. I tracked them down by following
>> the call chains leading to bdrv_open().
>
> Possibly also interesting for a followup: All the ways you can reopen
> BDSes with different options/flags.
Can do "all the ways you can reopen BDSes". Finding out whether and how
options or flags can be changed in each case would be too much for one
go, I'm afraid.
>> = QMP Commands =
>>
>> * block-commit
>>
>> I figure this clones the @base BDS initially, and replaces it by the
>> clone finally. Is support for user configuration for this clone
>> wanted?
>
> I don't think such a clone exists. Data is directly committed to @base.
> The command looks to me as if it could be okay.
Here's what made me speculate about clones. Call chain:
qmp_block_commit()
commit_active_start()
mirror_start_job(), passing commit_active_job_driver
On completion:
block_job_complete()
mirror_complete(), through commit_active_job_driver.complete
bdrv_open_backing_file()
bdrv_open()
I now see that my reading wasn't correct. But I can't see offhand what
exactly happens here. Can you enlighten me?
>> * blockdev-add
>>
>> Boils down to a bdrv_open(), which is discussed in the next section.
>>
>> * blockdev-snapshot-sync
>>
>> Creates a new image and a new BDS with only a few configuration
>> options: @snapshot-file (the filename), @snapshot-node-name,
>> @format=qcow2, @mode. Conflates three operations: create image, open
>> image, snapshot. I guess we want to replace it by a basic snapshot
>> operation that can be used with with blockdev-add and some command to
>> create images.
>
> Yes. We should have called this one drive-snapshot, it fits better in
> the drive-* family of commands. We can call the real blockdev-style
> command blockdev-snapshot - it is still synchronous technically, but it
> doesn't do anything like bdrv_open() that could be blocking.
Yes.
>> * change
>>
>> Closes, then opens a BDS with just two configuration options: @target
>> (the filename) and @arg (the format). Needs replacing.
>
> Max (added to CC) is working on it.
>
>> * drive-backup
>>
>> Similar to blockdev-snapshot-sync, except the filename is called
>> @target, and the node name can't be configured. I guess we want to
>> replace it by a basic backup operation.
>>
>> * drive-mirror
>>
>> Similar to blockdev-snapshot-sync, except the filename is called
>> @target, and the node name @node-name. I guess we want to replace it
>> by a basic mirror operation.
>
> Yes. We called these drive-* instead of blockdev-* intentionally, so
> that the latter names would be free for operations working on existing
> BDSes.
>
>> * transaction
>>
>> This is a wrapper around a list of transaction-capable commands.
>> Thus, nothing new here.
>>
>>
>> = Generic block layer =
>>
>> bdrv_open() opens a BDS and possibly children "file" and "backing"
>> according to its configuration.
>>
>> Subtypes of BlockdevOptionsGenericFormat have configuration for "file".
>>
>> Subtypes of BlockdevOptionsGenericCOWFormat additionally have
>> configuration for "backing" (defaults to "infer from COW image").
>>
>> bdrv_open() can additionally splice in a QCOW2 BDS to implement
>> snapshot=on. No way to configure, but that's okay, because it's a
>> convenience feature, and to configure you simply do the splicing
>> explicitly.
>>
>>
>> = Block driver methods =
>>
>> == bdrv_create() ==
>>
>> The BDSes created here are all internal temporaries of the method, hence
>> user configuration isn't needed. Correct?
>
> Filenames ought to be enough for everyone. Not.
>
> But at the moment all the callers can't deal with non-filename
Re: [Qemu-devel] [PATCH 1/2] tcg: Introduce tcg_arch_init() function
On 12/18/2014 10:26 PM, Eduardo Habkost wrote: > +void tcg_arch_init(void); I'm not keen on the name, because it's fairly ambiguous whether "arch" is talking about the host or target. It doesn't help that "tcg_target" is the host. ;-P Assuming we do this (see hook discussion), what about "translator" instead? r~
Re: [Qemu-devel] [Qemu-ppc] Unable to loadvm on qemu-system-ppc -M g3beige (keyboard freeze)
On 19/12/14 12:29, Peter Maydell wrote: > On 19 December 2014 at 10:53, Mark Cave-Ayland > wrote: >> It seems that I've misunderstood something mentioned earlier in the >> thread, i.e. that loadvm also does the create, initialize, realize and >> reset cycle. At least issuing "loadvm" in the monitor doesn't seem to do >> that as far as I can see here? > > For monitor "loadvm", we've already done create/initialize/realize > as part of our initial setup of the VM when we started QEMU. > reset is done by load_vmstate() calling qemu_system_reset(). > > For command line "-loadvm", we do the load_vmstate() after the > initial reset in vl.c. > > Either way, VM reloading doesn't need to deal with anything that's > set up in device init/realize as a fixed config, and it doesn't > need to assert IRQ lines either. I now have something that nearly works in that I can issue a "savevm" followed by a "loadvm" in the monitor and be able to resume where I left off - but only in OpenBIOS. If I boot into an OS and try this then it doesn't work and the console just sits there frozen. Also there seems to be a difference between issuing "loadvm" in the monitor which works, and adding -loadvm to the command line which completes successfully but still doesn't work as the console remains frozen, even just in OpenBIOS. This looks similar to trying to load an image created within an OS as above. I poked around with gdbstub and the difference with -loadvm on the command line is that as soon as I step into the next instruction I get a DSI at the PC address within OpenBIOS space which can't be resolved. According to "info mtree" the BIOS is present at the physical address, but it appears that the 1:1 physical to virtual mapping for the BIOS isn't there (or at least both gdbstub and trying to access the virtual address where the BIOS is supposed to be located gives a "Cannot access memory" error). Could it be that some of the CPU TLB state doesn't get serialized to disk as part of "savevm" which is causing these immediate faults when launching with -loadvm? ATB, Mark.
[Qemu-devel] Live migration bug, possible missing ram in migration?
Hi, I've been using kvm for some time now using live migration as well with ceph backend. Recently I started running into an issue with only one of my VM, which happens to be a windows server (2012). When I migrate this particular VM it seems that not all the ram is transferred. So when the migration completes, the vm that has been migrated simply hangs and I have to force a shutdown. Notice that not long ago it was working fine, however I didn't notice when it started to have the issue. Notice that my diagnostic of the problem may be wrong, I mean the behavior i'm having is that my VM hangs after live migration, and I don't see the problem on other VMs. But I noticed the difference in RAM usage by the qemu process which leads me to believe that the ram is not fully transferred. before migration on node2: 15225 libvirt+ 20 0 7128048 4.200g 13592 S 15.0 27.3 1:14.18 qemu-system-x86 libvirt+ 15225 52.7 27.3 7128048 4404244 ? Sl 12:57 1:14 qemu-system-x86_64 after migration on node1: 16507 libvirt+ 20 0 6571864 1.610g 13152 R 7.6 10.5 0:07.63 qemu-system-x86 libvirt+ 16507 15.7 10.4 6571864 1688392 ? Sl 13:00 0:08 qemu-system-x86_64 libvirtd.log on node02: 2014-12-19 13:01:06.654+: 6845: warning : qemuMigrationCancelDriveMirror:1421 : Unable to stop block job on drive-virtio-disk0 2014-12-19 13:01:06.656+: 6845: warning : qemuMigrationCancelDriveMirror:1421 : Unable to stop block job on drive-virtio-disk1 libvirtd.log on node01: 2014-12-19 13:00:52.346+: 7258: warning : qemuDomainObjEnterMonitorInternal:1274 : This thread seems to be the async job owner; entering monitor without asking for a nested job is dangerous 2014-12-19 13:00:52.436+: 7258: warning : qemuDomainObjEnterMonitorInternal:1274 : This thread seems to be the async job owner; entering monitor without asking for a nested job is dangerous 2014-12-19 13:00:52.437+: 7258: warning : qemuDomainObjEnterMonitorInternal:1274 : This thread seems to be the async job owner; entering monitor without asking for a nested job is dangerous 2014-12-19 13:00:52.441+: 7258: warning : qemuDomainObjEnterMonitorInternal:1274 : This thread seems to be the async job owner; entering monitor without asking for a nested job is dangerous 2014-12-19 13:00:52.480+: 7258: warning : qemuDomainObjEnterMonitorInternal:1274 : This thread seems to be the async job owner; entering monitor without asking for a nested job is dangerous 2014-12-19 13:00:52.480+: 7258: warning : qemuDomainObjEnterMonitorInternal:1274 : This thread seems to be the async job owner; entering monitor without asking for a nested job is dangerous 2014-12-19 13:00:52.481+: 7258: warning : qemuDomainObjEnterMonitorInternal:1274 : This thread seems to be the async job owner; entering monitor without asking for a nested job is dangerous ceph version 0.87-1trusty ubuntu 14.04 LTS Linux compute02 3.13.0-43-generic #72-Ubuntu SMP Mon Dec 8 19:35:06 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux (on both nodes) virsh cmd to migrate: virsh migrate --live win12 qemu+ssh://192.x.x.x/system echo $? 0 qemu cmd line before on node2: 30308 ?Sl 449:03 qemu-system-x86_64 -enable-kvm -name win12 -S -machine pc-i440fx-trusty,accel=kvm,usb=off -cpu Nehalem -m 4096 -realtime mlock=off -smp 4,sockets=4,cores=1,threads=1 -uuid be2c1183-de3b-4994-a20b-2b89a9c4b073 -no-user-config -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/win12.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=localtime,driftfix=slew -global kvm-pit.lost_tick_policy=discard -no-hpet -no-shutdown -global PIIX4_PM.disable_s3=1 -global PIIX4_PM.disable_s4=1 -boot menu=off,strict=on -device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x5.0x7 -device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x5 -device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x5.0x1 -device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x5.0x2 -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x6 -drive file=rbd:libvirt-pool/win12:id=libvirt:key=AQCKS4ZTQMYDKBAA1Q8zuys/l+OJ/n9GJjlk9g==:auth_supported=cephx\;none:mon_host=compute01\:6789\;compute02\:6789\;mgmt01\:6789,if=none,id=drive-virtio-disk0,cache=writeback -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x8,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 -drive file=rbd:live_migration/win2012_backups:id=live_migration:key=AQCyRsRRaI6IAxAAPW74dBKlAVUJvkaXadaecw==:auth_supported=cephx\;none:mon_host=compute01\:6789\;compute02\:6789\;mgmt01\:6789,if=none,id=drive-virtio-disk1 -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x9,drive=drive-virtio-disk1,id=virtio-disk1 -drive if=none,id=drive-ide0-0-1,readonly=on,format=raw -device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 -netdev tap,fd=26,id=hostnet0,vhost=on,vhostfd=27 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:fa:aa:c6,bus=pci.0,addr=0x3 -chardev pty,id=charserial0
