| Message ID | 20190514151848.5677-1-yury-kotov@yandex-team.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] monitor: Fix fdset_id & fd types for corresponding QMP commands | expand |
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
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
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;
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(-)