include/monitor/monitor.h | 6 +++--- monitor.c | 18 +++++++++--------- qapi/misc.json | 10 +++++----- stubs/fdset.c | 4 ++-- util/osdep.c | 4 ++-- vl.c | 2 +- 6 files changed, 22 insertions(+), 22 deletions(-)
Now, fdset_id is int64, but in some places we work with it as int.
It seems that there is no sense to use int64 for fdset_id, so it's
better to fix inconsistency by changing fdset_id type to int and by
fixing the reference of corresponding QMP commands: add-fd, remove-fd,
query-fdsets.
Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>
---
include/monitor/monitor.h | 6 +++---
monitor.c | 18 +++++++++---------
qapi/misc.json | 10 +++++-----
stubs/fdset.c | 4 ++--
util/osdep.c | 4 ++--
vl.c | 2 +-
6 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 86656297f1..06f9b6c217 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -39,11 +39,11 @@ void monitor_read_command(Monitor *mon, int show_prompt);
int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
void *opaque);
-AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
+AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int fdset_id,
bool has_opaque, const char *opaque,
Error **errp);
-int monitor_fdset_get_fd(int64_t fdset_id, int flags);
-int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd);
+int monitor_fdset_get_fd(int fdset_id, int flags);
+int monitor_fdset_dup_fd_add(int fdset_id, int dup_fd);
void monitor_fdset_dup_fd_remove(int dup_fd);
int monitor_fdset_dup_fd_find(int dup_fd);
diff --git a/monitor.c b/monitor.c
index bb48997913..b71ce816bc 100644
--- a/monitor.c
+++ b/monitor.c
@@ -160,7 +160,7 @@ struct MonFdsetFd {
/* file descriptor set containing fds passed via SCM_RIGHTS */
typedef struct MonFdset MonFdset;
struct MonFdset {
- int64_t id;
+ int id;
QLIST_HEAD(, MonFdsetFd) fds;
QLIST_HEAD(, MonFdsetFd) dup_fds;
QLIST_ENTRY(MonFdset) next;
@@ -2346,7 +2346,7 @@ static void monitor_fdsets_cleanup(void)
qemu_mutex_unlock(&mon_fdsets_lock);
}
-AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, bool has_opaque,
+AddfdInfo *qmp_add_fd(bool has_fdset_id, int32_t fdset_id, bool has_opaque,
const char *opaque, Error **errp)
{
int fd;
@@ -2372,7 +2372,7 @@ error:
return NULL;
}
-void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp)
+void qmp_remove_fd(int32_t fdset_id, bool has_fd, int fd, Error **errp)
{
MonFdset *mon_fdset;
MonFdsetFd *mon_fdset_fd;
@@ -2405,10 +2405,10 @@ void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp)
error:
qemu_mutex_unlock(&mon_fdsets_lock);
if (has_fd) {
- snprintf(fd_str, sizeof(fd_str), "fdset-id:%" PRId64 ", fd:%" PRId64,
+ snprintf(fd_str, sizeof(fd_str), "fdset-id:%" PRId32 ", fd:%" PRId32,
fdset_id, fd);
} else {
- snprintf(fd_str, sizeof(fd_str), "fdset-id:%" PRId64, fdset_id);
+ snprintf(fd_str, sizeof(fd_str), "fdset-id:%" PRId32, fdset_id);
}
error_setg(errp, QERR_FD_NOT_FOUND, fd_str);
}
@@ -2454,7 +2454,7 @@ FdsetInfoList *qmp_query_fdsets(Error **errp)
return fdset_list;
}
-AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
+AddfdInfo *monitor_fdset_add_fd(int32_t fd, bool has_fdset_id, int32_t fdset_id,
bool has_opaque, const char *opaque,
Error **errp)
{
@@ -2476,7 +2476,7 @@ AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
}
if (mon_fdset == NULL) {
- int64_t fdset_id_prev = -1;
+ int fdset_id_prev = -1;
MonFdset *mon_fdset_cur = QLIST_FIRST(&mon_fdsets);
if (has_fdset_id) {
@@ -2538,7 +2538,7 @@ AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
return fdinfo;
}
-int monitor_fdset_get_fd(int64_t fdset_id, int flags)
+int monitor_fdset_get_fd(int fdset_id, int flags)
{
#ifdef _WIN32
return -ENOENT;
@@ -2576,7 +2576,7 @@ out:
#endif
}
-int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd)
+int monitor_fdset_dup_fd_add(int fdset_id, int dup_fd)
{
MonFdset *mon_fdset;
MonFdsetFd *mon_fdset_fd_dup;
diff --git a/qapi/misc.json b/qapi/misc.json
index 8b3ca4fdd3..b345e1458b 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -2179,7 +2179,7 @@
#
# Since: 1.2.0
##
-{ 'struct': 'AddfdInfo', 'data': {'fdset-id': 'int', 'fd': 'int'} }
+{ 'struct': 'AddfdInfo', 'data': {'fdset-id': 'int32', 'fd': 'int32'} }
##
# @add-fd:
@@ -2209,7 +2209,7 @@
#
##
{ 'command': 'add-fd',
- 'data': { '*fdset-id': 'int',
+ 'data': { '*fdset-id': 'int32',
'*opaque': 'str' },
'returns': 'AddfdInfo' }
@@ -2238,7 +2238,7 @@
# <- { "return": {} }
#
##
-{ 'command': 'remove-fd', 'data': {'fdset-id': 'int', '*fd': 'int'} }
+{ 'command': 'remove-fd', 'data': {'fdset-id': 'int32', '*fd': 'int32'} }
##
# @FdsetFdInfo:
@@ -2252,7 +2252,7 @@
# Since: 1.2.0
##
{ 'struct': 'FdsetFdInfo',
- 'data': {'fd': 'int', '*opaque': 'str'} }
+ 'data': {'fd': 'int32', '*opaque': 'str'} }
##
# @FdsetInfo:
@@ -2266,7 +2266,7 @@
# Since: 1.2.0
##
{ 'struct': 'FdsetInfo',
- 'data': {'fdset-id': 'int', 'fds': ['FdsetFdInfo']} }
+ 'data': {'fdset-id': 'int32', 'fds': ['FdsetFdInfo']} }
##
# @query-fdsets:
diff --git a/stubs/fdset.c b/stubs/fdset.c
index 4f3edf2ea4..1504624c19 100644
--- a/stubs/fdset.c
+++ b/stubs/fdset.c
@@ -2,7 +2,7 @@
#include "qemu-common.h"
#include "monitor/monitor.h"
-int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd)
+int monitor_fdset_dup_fd_add(int fdset_id, int dup_fd)
{
return -1;
}
@@ -12,7 +12,7 @@ int monitor_fdset_dup_fd_find(int dup_fd)
return -1;
}
-int monitor_fdset_get_fd(int64_t fdset_id, int flags)
+int monitor_fdset_get_fd(int fdset_id, int flags)
{
return -ENOENT;
}
diff --git a/util/osdep.c b/util/osdep.c
index 3f04326040..9e2d3768e0 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -292,7 +292,7 @@ int qemu_open(const char *name, int flags, ...)
/* Attempt dup of fd from fd set */
if (strstart(name, "/dev/fdset/", &fdset_id_str)) {
- int64_t fdset_id;
+ int fdset_id;
int fd, dupfd;
fdset_id = qemu_parse_fdset(fdset_id_str);
@@ -352,7 +352,7 @@ int qemu_open(const char *name, int flags, ...)
int qemu_close(int fd)
{
- int64_t fdset_id;
+ int fdset_id;
/* Close fd that was dup'd from an fdset */
fdset_id = monitor_fdset_dup_fd_find(fd);
diff --git a/vl.c b/vl.c
index b6709514c1..0f5622496c 100644
--- a/vl.c
+++ b/vl.c
@@ -1081,7 +1081,7 @@ bool defaults_enabled(void)
static int parse_add_fd(void *opaque, QemuOpts *opts, Error **errp)
{
int fd, dupfd, flags;
- int64_t fdset_id;
+ int fdset_id;
const char *fd_opaque = NULL;
AddfdInfo *fdinfo;
--
2.21.0
Yury Kotov <yury-kotov@yandex-team.ru> writes:
> Now, fdset_id is int64, but in some places we work with it as int.
> It seems that there is no sense to use int64 for fdset_id, so it's
> better to fix inconsistency by changing fdset_id type to int and by
> fixing the reference of corresponding QMP commands: add-fd, remove-fd,
> query-fdsets.
>
> Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>
> ---
> include/monitor/monitor.h | 6 +++---
> monitor.c | 18 +++++++++---------
> qapi/misc.json | 10 +++++-----
> stubs/fdset.c | 4 ++--
> util/osdep.c | 4 ++--
> vl.c | 2 +-
> 6 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> index 86656297f1..06f9b6c217 100644
> --- a/include/monitor/monitor.h
> +++ b/include/monitor/monitor.h
> @@ -39,11 +39,11 @@ void monitor_read_command(Monitor *mon, int show_prompt);
> int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
> void *opaque);
>
> -AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
> +AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int fdset_id,
> bool has_opaque, const char *opaque,
> Error **errp);
> -int monitor_fdset_get_fd(int64_t fdset_id, int flags);
> -int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd);
> +int monitor_fdset_get_fd(int fdset_id, int flags);
> +int monitor_fdset_dup_fd_add(int fdset_id, int dup_fd);
> void monitor_fdset_dup_fd_remove(int dup_fd);
> int monitor_fdset_dup_fd_find(int dup_fd);
>
> diff --git a/monitor.c b/monitor.c
> index bb48997913..b71ce816bc 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -160,7 +160,7 @@ struct MonFdsetFd {
> /* file descriptor set containing fds passed via SCM_RIGHTS */
> typedef struct MonFdset MonFdset;
> struct MonFdset {
> - int64_t id;
> + int id;
In C, you use int instead of int64_t for fdset IDs.
> QLIST_HEAD(, MonFdsetFd) fds;
> QLIST_HEAD(, MonFdsetFd) dup_fds;
> QLIST_ENTRY(MonFdset) next;
> @@ -2346,7 +2346,7 @@ static void monitor_fdsets_cleanup(void)
> qemu_mutex_unlock(&mon_fdsets_lock);
> }
>
> -AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, bool has_opaque,
> +AddfdInfo *qmp_add_fd(bool has_fdset_id, int32_t fdset_id, bool has_opaque,
> const char *opaque, Error **errp)
> {
> int fd;
> @@ -2372,7 +2372,7 @@ error:
> return NULL;
> }
>
> -void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp)
> +void qmp_remove_fd(int32_t fdset_id, bool has_fd, int fd, Error **errp)
> {
> MonFdset *mon_fdset;
> MonFdsetFd *mon_fdset_fd;
> @@ -2405,10 +2405,10 @@ void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp)
> error:
> qemu_mutex_unlock(&mon_fdsets_lock);
> if (has_fd) {
> - snprintf(fd_str, sizeof(fd_str), "fdset-id:%" PRId64 ", fd:%" PRId64,
> + snprintf(fd_str, sizeof(fd_str), "fdset-id:%" PRId32 ", fd:%" PRId32,
> fdset_id, fd);
> } else {
> - snprintf(fd_str, sizeof(fd_str), "fdset-id:%" PRId64, fdset_id);
> + snprintf(fd_str, sizeof(fd_str), "fdset-id:%" PRId32, fdset_id);
> }
> error_setg(errp, QERR_FD_NOT_FOUND, fd_str);
> }
> @@ -2454,7 +2454,7 @@ FdsetInfoList *qmp_query_fdsets(Error **errp)
> return fdset_list;
> }
>
> -AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
> +AddfdInfo *monitor_fdset_add_fd(int32_t fd, bool has_fdset_id, int32_t fdset_id,
> bool has_opaque, const char *opaque,
> Error **errp)
> {
> @@ -2476,7 +2476,7 @@ AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
> }
>
> if (mon_fdset == NULL) {
> - int64_t fdset_id_prev = -1;
> + int fdset_id_prev = -1;
> MonFdset *mon_fdset_cur = QLIST_FIRST(&mon_fdsets);
>
> if (has_fdset_id) {
> @@ -2538,7 +2538,7 @@ AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
> return fdinfo;
> }
>
> -int monitor_fdset_get_fd(int64_t fdset_id, int flags)
> +int monitor_fdset_get_fd(int fdset_id, int flags)
> {
> #ifdef _WIN32
> return -ENOENT;
> @@ -2576,7 +2576,7 @@ out:
> #endif
> }
>
> -int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd)
> +int monitor_fdset_dup_fd_add(int fdset_id, int dup_fd)
> {
> MonFdset *mon_fdset;
> MonFdsetFd *mon_fdset_fd_dup;
> diff --git a/qapi/misc.json b/qapi/misc.json
> index 8b3ca4fdd3..b345e1458b 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -2179,7 +2179,7 @@
> #
> # Since: 1.2.0
> ##
> -{ 'struct': 'AddfdInfo', 'data': {'fdset-id': 'int', 'fd': 'int'} }
> +{ 'struct': 'AddfdInfo', 'data': {'fdset-id': 'int32', 'fd': 'int32'} }
>
> ##
> # @add-fd:
> @@ -2209,7 +2209,7 @@
> #
> ##
> { 'command': 'add-fd',
> - 'data': { '*fdset-id': 'int',
> + 'data': { '*fdset-id': 'int32',
> '*opaque': 'str' },
> 'returns': 'AddfdInfo' }
>
> @@ -2238,7 +2238,7 @@
> # <- { "return": {} }
> #
> ##
> -{ 'command': 'remove-fd', 'data': {'fdset-id': 'int', '*fd': 'int'} }
> +{ 'command': 'remove-fd', 'data': {'fdset-id': 'int32', '*fd': 'int32'} }
>
> ##
> # @FdsetFdInfo:
> @@ -2252,7 +2252,7 @@
> # Since: 1.2.0
> ##
> { 'struct': 'FdsetFdInfo',
> - 'data': {'fd': 'int', '*opaque': 'str'} }
> + 'data': {'fd': 'int32', '*opaque': 'str'} }
>
> ##
> # @FdsetInfo:
> @@ -2266,7 +2266,7 @@
> # Since: 1.2.0
> ##
> { 'struct': 'FdsetInfo',
> - 'data': {'fdset-id': 'int', 'fds': ['FdsetFdInfo']} }
> + 'data': {'fdset-id': 'int32', 'fds': ['FdsetFdInfo']} }
>
> ##
> # @query-fdsets:
In the schema, you use QAPI type 'int32' instead of 'int'.
Before the patch, the two are consistent (except for the bugs you fixed
in v1 of this patch): QAPI 'int' is C int64_t.
After the patch, the two are inconsistent: QAPI 'int32' is C int32_t,
not int. They're usually the same, but it unclean.
Two ways forward:
1. Revise this patch to use int32_t instead of int.
2. Revise v1 to address the few minor review comments I had. Smaller
patch, easier to review.
Your choice. I'd choose 2.
[...]
22.05.2019, 19:40, "Markus Armbruster" <armbru@redhat.com>:
> Yury Kotov <yury-kotov@yandex-team.ru> writes:
>
>> Now, fdset_id is int64, but in some places we work with it as int.
>> It seems that there is no sense to use int64 for fdset_id, so it's
>> better to fix inconsistency by changing fdset_id type to int and by
>> fixing the reference of corresponding QMP commands: add-fd, remove-fd,
>> query-fdsets.
>>
>> Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>
>> ---
>> include/monitor/monitor.h | 6 +++---
>> monitor.c | 18 +++++++++---------
>> qapi/misc.json | 10 +++++-----
>> stubs/fdset.c | 4 ++--
>> util/osdep.c | 4 ++--
>> vl.c | 2 +-
>> 6 files changed, 22 insertions(+), 22 deletions(-)
>>
>> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
>> index 86656297f1..06f9b6c217 100644
>> --- a/include/monitor/monitor.h
>> +++ b/include/monitor/monitor.h
>> @@ -39,11 +39,11 @@ void monitor_read_command(Monitor *mon, int show_prompt);
>> int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
>> void *opaque);
>>
>> -AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
>> +AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int fdset_id,
>> bool has_opaque, const char *opaque,
>> Error **errp);
>> -int monitor_fdset_get_fd(int64_t fdset_id, int flags);
>> -int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd);
>> +int monitor_fdset_get_fd(int fdset_id, int flags);
>> +int monitor_fdset_dup_fd_add(int fdset_id, int dup_fd);
>> void monitor_fdset_dup_fd_remove(int dup_fd);
>> int monitor_fdset_dup_fd_find(int dup_fd);
>>
>> diff --git a/monitor.c b/monitor.c
>> index bb48997913..b71ce816bc 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -160,7 +160,7 @@ struct MonFdsetFd {
>> /* file descriptor set containing fds passed via SCM_RIGHTS */
>> typedef struct MonFdset MonFdset;
>> struct MonFdset {
>> - int64_t id;
>> + int id;
>
> In C, you use int instead of int64_t for fdset IDs.
>
>> QLIST_HEAD(, MonFdsetFd) fds;
>> QLIST_HEAD(, MonFdsetFd) dup_fds;
>> QLIST_ENTRY(MonFdset) next;
>> @@ -2346,7 +2346,7 @@ static void monitor_fdsets_cleanup(void)
>> qemu_mutex_unlock(&mon_fdsets_lock);
>> }
>>
>> -AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, bool has_opaque,
>> +AddfdInfo *qmp_add_fd(bool has_fdset_id, int32_t fdset_id, bool has_opaque,
>> const char *opaque, Error **errp)
>> {
>> int fd;
>> @@ -2372,7 +2372,7 @@ error:
>> return NULL;
>> }
>>
>> -void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp)
>> +void qmp_remove_fd(int32_t fdset_id, bool has_fd, int fd, Error **errp)
>> {
>> MonFdset *mon_fdset;
>> MonFdsetFd *mon_fdset_fd;
>> @@ -2405,10 +2405,10 @@ void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp)
>> error:
>> qemu_mutex_unlock(&mon_fdsets_lock);
>> if (has_fd) {
>> - snprintf(fd_str, sizeof(fd_str), "fdset-id:%" PRId64 ", fd:%" PRId64,
>> + snprintf(fd_str, sizeof(fd_str), "fdset-id:%" PRId32 ", fd:%" PRId32,
>> fdset_id, fd);
>> } else {
>> - snprintf(fd_str, sizeof(fd_str), "fdset-id:%" PRId64, fdset_id);
>> + snprintf(fd_str, sizeof(fd_str), "fdset-id:%" PRId32, fdset_id);
>> }
>> error_setg(errp, QERR_FD_NOT_FOUND, fd_str);
>> }
>> @@ -2454,7 +2454,7 @@ FdsetInfoList *qmp_query_fdsets(Error **errp)
>> return fdset_list;
>> }
>>
>> -AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
>> +AddfdInfo *monitor_fdset_add_fd(int32_t fd, bool has_fdset_id, int32_t fdset_id,
>> bool has_opaque, const char *opaque,
>> Error **errp)
>> {
>> @@ -2476,7 +2476,7 @@ AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
>> }
>>
>> if (mon_fdset == NULL) {
>> - int64_t fdset_id_prev = -1;
>> + int fdset_id_prev = -1;
>> MonFdset *mon_fdset_cur = QLIST_FIRST(&mon_fdsets);
>>
>> if (has_fdset_id) {
>> @@ -2538,7 +2538,7 @@ AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
>> return fdinfo;
>> }
>>
>> -int monitor_fdset_get_fd(int64_t fdset_id, int flags)
>> +int monitor_fdset_get_fd(int fdset_id, int flags)
>> {
>> #ifdef _WIN32
>> return -ENOENT;
>> @@ -2576,7 +2576,7 @@ out:
>> #endif
>> }
>>
>> -int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd)
>> +int monitor_fdset_dup_fd_add(int fdset_id, int dup_fd)
>> {
>> MonFdset *mon_fdset;
>> MonFdsetFd *mon_fdset_fd_dup;
>> diff --git a/qapi/misc.json b/qapi/misc.json
>> index 8b3ca4fdd3..b345e1458b 100644
>> --- a/qapi/misc.json
>> +++ b/qapi/misc.json
>> @@ -2179,7 +2179,7 @@
>> #
>> # Since: 1.2.0
>> ##
>> -{ 'struct': 'AddfdInfo', 'data': {'fdset-id': 'int', 'fd': 'int'} }
>> +{ 'struct': 'AddfdInfo', 'data': {'fdset-id': 'int32', 'fd': 'int32'} }
>>
>> ##
>> # @add-fd:
>> @@ -2209,7 +2209,7 @@
>> #
>> ##
>> { 'command': 'add-fd',
>> - 'data': { '*fdset-id': 'int',
>> + 'data': { '*fdset-id': 'int32',
>> '*opaque': 'str' },
>> 'returns': 'AddfdInfo' }
>>
>> @@ -2238,7 +2238,7 @@
>> # <- { "return": {} }
>> #
>> ##
>> -{ 'command': 'remove-fd', 'data': {'fdset-id': 'int', '*fd': 'int'} }
>> +{ 'command': 'remove-fd', 'data': {'fdset-id': 'int32', '*fd': 'int32'} }
>>
>> ##
>> # @FdsetFdInfo:
>> @@ -2252,7 +2252,7 @@
>> # Since: 1.2.0
>> ##
>> { 'struct': 'FdsetFdInfo',
>> - 'data': {'fd': 'int', '*opaque': 'str'} }
>> + 'data': {'fd': 'int32', '*opaque': 'str'} }
>>
>> ##
>> # @FdsetInfo:
>> @@ -2266,7 +2266,7 @@
>> # Since: 1.2.0
>> ##
>> { 'struct': 'FdsetInfo',
>> - 'data': {'fdset-id': 'int', 'fds': ['FdsetFdInfo']} }
>> + 'data': {'fdset-id': 'int32', 'fds': ['FdsetFdInfo']} }
>>
>> ##
>> # @query-fdsets:
>
> In the schema, you use QAPI type 'int32' instead of 'int'.
>
> Before the patch, the two are consistent (except for the bugs you fixed
> in v1 of this patch): QAPI 'int' is C int64_t.
>
> After the patch, the two are inconsistent: QAPI 'int32' is C int32_t,
> not int. They're usually the same, but it unclean.
>
> Two ways forward:
>
> 1. Revise this patch to use int32_t instead of int.
>
> 2. Revise v1 to address the few minor review comments I had. Smaller
> patch, easier to review.
>
> Your choice. I'd choose 2.
>
> [...]
Sorry, I'm a little confused... I thought method 2 was not ok,
but if it’s not, then I prefer it too.
So, I will send v3 which is v1 with grammar fixes
Regards,
Yury
Ping
14.05.2019, 18:20, "Yury Kotov" <yury-kotov@yandex-team.ru>:
> Now, fdset_id is int64, but in some places we work with it as int.
> It seems that there is no sense to use int64 for fdset_id, so it's
> better to fix inconsistency by changing fdset_id type to int and by
> fixing the reference of corresponding QMP commands: add-fd, remove-fd,
> query-fdsets.
>
> Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>
> ---
> include/monitor/monitor.h | 6 +++---
> monitor.c | 18 +++++++++---------
> qapi/misc.json | 10 +++++-----
> stubs/fdset.c | 4 ++--
> util/osdep.c | 4 ++--
> vl.c | 2 +-
> 6 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> index 86656297f1..06f9b6c217 100644
> --- a/include/monitor/monitor.h
> +++ b/include/monitor/monitor.h
> @@ -39,11 +39,11 @@ void monitor_read_command(Monitor *mon, int show_prompt);
> int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
> void *opaque);
>
> -AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
> +AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int fdset_id,
> bool has_opaque, const char *opaque,
> Error **errp);
> -int monitor_fdset_get_fd(int64_t fdset_id, int flags);
> -int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd);
> +int monitor_fdset_get_fd(int fdset_id, int flags);
> +int monitor_fdset_dup_fd_add(int fdset_id, int dup_fd);
> void monitor_fdset_dup_fd_remove(int dup_fd);
> int monitor_fdset_dup_fd_find(int dup_fd);
>
> diff --git a/monitor.c b/monitor.c
> index bb48997913..b71ce816bc 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -160,7 +160,7 @@ struct MonFdsetFd {
> /* file descriptor set containing fds passed via SCM_RIGHTS */
> typedef struct MonFdset MonFdset;
> struct MonFdset {
> - int64_t id;
> + int id;
> QLIST_HEAD(, MonFdsetFd) fds;
> QLIST_HEAD(, MonFdsetFd) dup_fds;
> QLIST_ENTRY(MonFdset) next;
> @@ -2346,7 +2346,7 @@ static void monitor_fdsets_cleanup(void)
> qemu_mutex_unlock(&mon_fdsets_lock);
> }
>
> -AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, bool has_opaque,
> +AddfdInfo *qmp_add_fd(bool has_fdset_id, int32_t fdset_id, bool has_opaque,
> const char *opaque, Error **errp)
> {
> int fd;
> @@ -2372,7 +2372,7 @@ error:
> return NULL;
> }
>
> -void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp)
> +void qmp_remove_fd(int32_t fdset_id, bool has_fd, int fd, Error **errp)
> {
> MonFdset *mon_fdset;
> MonFdsetFd *mon_fdset_fd;
> @@ -2405,10 +2405,10 @@ void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp)
> error:
> qemu_mutex_unlock(&mon_fdsets_lock);
> if (has_fd) {
> - snprintf(fd_str, sizeof(fd_str), "fdset-id:%" PRId64 ", fd:%" PRId64,
> + snprintf(fd_str, sizeof(fd_str), "fdset-id:%" PRId32 ", fd:%" PRId32,
> fdset_id, fd);
> } else {
> - snprintf(fd_str, sizeof(fd_str), "fdset-id:%" PRId64, fdset_id);
> + snprintf(fd_str, sizeof(fd_str), "fdset-id:%" PRId32, fdset_id);
> }
> error_setg(errp, QERR_FD_NOT_FOUND, fd_str);
> }
> @@ -2454,7 +2454,7 @@ FdsetInfoList *qmp_query_fdsets(Error **errp)
> return fdset_list;
> }
>
> -AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
> +AddfdInfo *monitor_fdset_add_fd(int32_t fd, bool has_fdset_id, int32_t fdset_id,
> bool has_opaque, const char *opaque,
> Error **errp)
> {
> @@ -2476,7 +2476,7 @@ AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
> }
>
> if (mon_fdset == NULL) {
> - int64_t fdset_id_prev = -1;
> + int fdset_id_prev = -1;
> MonFdset *mon_fdset_cur = QLIST_FIRST(&mon_fdsets);
>
> if (has_fdset_id) {
> @@ -2538,7 +2538,7 @@ AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
> return fdinfo;
> }
>
> -int monitor_fdset_get_fd(int64_t fdset_id, int flags)
> +int monitor_fdset_get_fd(int fdset_id, int flags)
> {
> #ifdef _WIN32
> return -ENOENT;
> @@ -2576,7 +2576,7 @@ out:
> #endif
> }
>
> -int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd)
> +int monitor_fdset_dup_fd_add(int fdset_id, int dup_fd)
> {
> MonFdset *mon_fdset;
> MonFdsetFd *mon_fdset_fd_dup;
> diff --git a/qapi/misc.json b/qapi/misc.json
> index 8b3ca4fdd3..b345e1458b 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -2179,7 +2179,7 @@
> #
> # Since: 1.2.0
> ##
> -{ 'struct': 'AddfdInfo', 'data': {'fdset-id': 'int', 'fd': 'int'} }
> +{ 'struct': 'AddfdInfo', 'data': {'fdset-id': 'int32', 'fd': 'int32'} }
>
> ##
> # @add-fd:
> @@ -2209,7 +2209,7 @@
> #
> ##
> { 'command': 'add-fd',
> - 'data': { '*fdset-id': 'int',
> + 'data': { '*fdset-id': 'int32',
> '*opaque': 'str' },
> 'returns': 'AddfdInfo' }
>
> @@ -2238,7 +2238,7 @@
> # <- { "return": {} }
> #
> ##
> -{ 'command': 'remove-fd', 'data': {'fdset-id': 'int', '*fd': 'int'} }
> +{ 'command': 'remove-fd', 'data': {'fdset-id': 'int32', '*fd': 'int32'} }
>
> ##
> # @FdsetFdInfo:
> @@ -2252,7 +2252,7 @@
> # Since: 1.2.0
> ##
> { 'struct': 'FdsetFdInfo',
> - 'data': {'fd': 'int', '*opaque': 'str'} }
> + 'data': {'fd': 'int32', '*opaque': 'str'} }
>
> ##
> # @FdsetInfo:
> @@ -2266,7 +2266,7 @@
> # Since: 1.2.0
> ##
> { 'struct': 'FdsetInfo',
> - 'data': {'fdset-id': 'int', 'fds': ['FdsetFdInfo']} }
> + 'data': {'fdset-id': 'int32', 'fds': ['FdsetFdInfo']} }
>
> ##
> # @query-fdsets:
> diff --git a/stubs/fdset.c b/stubs/fdset.c
> index 4f3edf2ea4..1504624c19 100644
> --- a/stubs/fdset.c
> +++ b/stubs/fdset.c
> @@ -2,7 +2,7 @@
> #include "qemu-common.h"
> #include "monitor/monitor.h"
>
> -int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd)
> +int monitor_fdset_dup_fd_add(int fdset_id, int dup_fd)
> {
> return -1;
> }
> @@ -12,7 +12,7 @@ int monitor_fdset_dup_fd_find(int dup_fd)
> return -1;
> }
>
> -int monitor_fdset_get_fd(int64_t fdset_id, int flags)
> +int monitor_fdset_get_fd(int fdset_id, int flags)
> {
> return -ENOENT;
> }
> diff --git a/util/osdep.c b/util/osdep.c
> index 3f04326040..9e2d3768e0 100644
> --- a/util/osdep.c
> +++ b/util/osdep.c
> @@ -292,7 +292,7 @@ int qemu_open(const char *name, int flags, ...)
>
> /* Attempt dup of fd from fd set */
> if (strstart(name, "/dev/fdset/", &fdset_id_str)) {
> - int64_t fdset_id;
> + int fdset_id;
> int fd, dupfd;
>
> fdset_id = qemu_parse_fdset(fdset_id_str);
> @@ -352,7 +352,7 @@ int qemu_open(const char *name, int flags, ...)
>
> int qemu_close(int fd)
> {
> - int64_t fdset_id;
> + int fdset_id;
>
> /* Close fd that was dup'd from an fdset */
> fdset_id = monitor_fdset_dup_fd_find(fd);
> diff --git a/vl.c b/vl.c
> index b6709514c1..0f5622496c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1081,7 +1081,7 @@ bool defaults_enabled(void)
> static int parse_add_fd(void *opaque, QemuOpts *opts, Error **errp)
> {
> int fd, dupfd, flags;
> - int64_t fdset_id;
> + int fdset_id;
> const char *fd_opaque = NULL;
> AddfdInfo *fdinfo;
>
> --
> 2.21.0
© 2016 - 2024 Red Hat, Inc.