| Message ID | 1344014889-12390-3-git-send-email-coreyb@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Aug 3, 2012 at 6:28 PM, Corey Bryant <coreyb@linux.vnet.ibm.com> wrote: > diff --git a/monitor.c b/monitor.c > index 49dccfe..9aa9f7e 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -140,6 +140,24 @@ struct mon_fd_t { > QLIST_ENTRY(mon_fd_t) next; > }; > > +/* file descriptor associated with a file descriptor set */ > +typedef struct mon_fdset_fd_t mon_fdset_fd_t; > +struct mon_fdset_fd_t { QEMU coding style is: typedef struct MonFdsetFd MonFdsetFd; struct MonFdsetFd { See ./CODING_STYLE for more info. > + int fd; > + bool removed; > + QLIST_ENTRY(mon_fdset_fd_t) next; > +}; > + > +/* file descriptor set containing fds passed via SCM_RIGHTS */ > +typedef struct mon_fdset_t mon_fdset_t; > +struct mon_fdset_t { > + int64_t id; > + int refcount; > + bool in_use; > + QLIST_HEAD(, mon_fdset_fd_t) fds; > + QLIST_ENTRY(mon_fdset_t) next; > +}; At this point in the patch series it's not clear to me whether the removed and refcount/in_use fields are a clean and correct solution. Exposing these fields via QMP is also something I'm going to carefully review because they seem like internals. > + > typedef struct MonitorControl { > QObject *id; > JSONMessageParser parser; > @@ -176,7 +194,8 @@ struct Monitor { > int print_calls_nr; > #endif > QError *error; > - QLIST_HEAD(,mon_fd_t) fds; > + QLIST_HEAD(, mon_fd_t) fds; > + QLIST_HEAD(, mon_fdset_t) fdsets; > QLIST_ENTRY(Monitor) entry; > }; > > @@ -2389,6 +2408,157 @@ int monitor_get_fd(Monitor *mon, const char *fdname) > return -1; > } > > +static void monitor_fdset_cleanup(mon_fdset_t *mon_fdset) > +{ > + mon_fdset_fd_t *mon_fdset_fd; > + mon_fdset_fd_t *mon_fdset_fd_next; > + > + if (mon_fdset->refcount != 0) { > + return; > + } > + > + QLIST_FOREACH_SAFE(mon_fdset_fd, &mon_fdset->fds, next, mon_fdset_fd_next) { > + if (!mon_fdset->in_use || mon_fdset_fd->removed) { > + close(mon_fdset_fd->fd); > + QLIST_REMOVE(mon_fdset_fd, next); > + g_free(mon_fdset_fd); > + } > + } > + > + if (QLIST_EMPTY(&mon_fdset->fds)) { > + QLIST_REMOVE(mon_fdset, next); > + g_free(mon_fdset); > + } > +} > + > +AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, Error **errp) > +{ > + int fd; > + Monitor *mon = cur_mon; > + mon_fdset_t *mon_fdset; > + mon_fdset_fd_t *mon_fdset_fd; > + AddfdInfo *fdinfo; > + > + fd = qemu_chr_fe_get_msgfd(mon->chr); > + if (fd == -1) { > + qerror_report(QERR_FD_NOT_SUPPLIED); > + return NULL; > + } > + > + if (has_fdset_id) { > + QLIST_FOREACH(mon_fdset, &mon->fdsets, next) { > + if (mon_fdset->id == fdset_id) { > + break; > + } > + } > + if (mon_fdset == NULL) { > + qerror_report(QERR_FDSET_NOT_FOUND, fdset_id); > + return NULL; fd is leaked? > + } > + } else { > + int64_t fdset_id_prev = -1; > + mon_fdset_t *mon_fdset_cur = QLIST_FIRST(&mon->fdsets); > + > + /* Use first available fdset ID */ > + QLIST_FOREACH(mon_fdset, &mon->fdsets, next) { > + mon_fdset_cur = mon_fdset; > + if (fdset_id_prev == mon_fdset_cur->id - 1) { > + fdset_id_prev = mon_fdset_cur->id; > + continue; > + } > + break; > + } > + > + mon_fdset = g_malloc0(sizeof(*mon_fdset)); > + mon_fdset->id = fdset_id_prev + 1; > + mon_fdset->refcount = 0; > + mon_fdset->in_use = true; > + > + /* The fdset list is ordered by fdset ID */ > + if (mon_fdset->id == 0) { > + QLIST_INSERT_HEAD(&mon->fdsets, mon_fdset, next); > + } else if (mon_fdset->id < mon_fdset_cur->id) { > + QLIST_INSERT_BEFORE(mon_fdset_cur, mon_fdset, next); > + } else { > + QLIST_INSERT_AFTER(mon_fdset_cur, mon_fdset, next); > + } > + } > + > + mon_fdset_fd = g_malloc0(sizeof(*mon_fdset_fd)); > + mon_fdset_fd->fd = fd; > + mon_fdset_fd->removed = false; > + QLIST_INSERT_HEAD(&mon_fdset->fds, mon_fdset_fd, next); > + > + fdinfo = g_malloc0(sizeof(*fdinfo)); > + fdinfo->fdset_id = mon_fdset->id; > + fdinfo->fd = mon_fdset_fd->fd; > + > + return fdinfo; > +} > + > +void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp) > +{ > + Monitor *mon = cur_mon; > + mon_fdset_t *mon_fdset; > + mon_fdset_fd_t *mon_fdset_fd; > + char fd_str[20]; > + > + QLIST_FOREACH(mon_fdset, &mon->fdsets, next) { > + if (mon_fdset->id != fdset_id) { > + continue; > + } > + QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) { > + if (has_fd && mon_fdset_fd->fd != fd) { > + continue; > + } > + mon_fdset_fd->removed = true; > + if (has_fd) { > + break; > + } > + } > + monitor_fdset_cleanup(mon_fdset); > + return; > + } > + snprintf(fd_str, sizeof(fd_str), "%ld", fd); > + qerror_report(QERR_FD_NOT_FOUND, fd_str); Why use an fd_str instead of passing an int64_t into the error message? This also assumed sizeof(long) == 8, which isn't true on 32-bit hosts, so %ld should be %"PRId64". There is a new policy on error reporting. I think this patch series may be affected/conflict, please qemu-devel to check. I think Luiz can also help here. Stefan
On 08/07/2012 02:16 PM, Stefan Hajnoczi wrote: > On Fri, Aug 3, 2012 at 6:28 PM, Corey Bryant <coreyb@linux.vnet.ibm.com> wrote: >> diff --git a/monitor.c b/monitor.c >> index 49dccfe..9aa9f7e 100644 >> --- a/monitor.c >> +++ b/monitor.c >> @@ -140,6 +140,24 @@ struct mon_fd_t { >> QLIST_ENTRY(mon_fd_t) next; >> }; >> >> +/* file descriptor associated with a file descriptor set */ >> +typedef struct mon_fdset_fd_t mon_fdset_fd_t; >> +struct mon_fdset_fd_t { > > QEMU coding style is: > > typedef struct MonFdsetFd MonFdsetFd; > struct MonFdsetFd { > > See ./CODING_STYLE for more info. > Thanks, I'll fix that. >> + int fd; >> + bool removed; >> + QLIST_ENTRY(mon_fdset_fd_t) next; >> +}; >> + >> +/* file descriptor set containing fds passed via SCM_RIGHTS */ >> +typedef struct mon_fdset_t mon_fdset_t; >> +struct mon_fdset_t { >> + int64_t id; >> + int refcount; >> + bool in_use; >> + QLIST_HEAD(, mon_fdset_fd_t) fds; >> + QLIST_ENTRY(mon_fdset_t) next; >> +}; > > At this point in the patch series it's not clear to me whether the > removed and refcount/in_use fields are a clean and correct solution. > Exposing these fields via QMP is also something I'm going to carefully > review because they seem like internals. > Yes, please review the v7 patches and let me know what you think. I explained the purpose of these fields in the previous email I just sent you, so I won't go into their details again here. But I will point out that refcount/in-use came about after concern of fd leakage if libvirt's monitor connection disconnects. query-fdsets allows the client to determine the state of the fd sets after reconnecting. >> + >> typedef struct MonitorControl { >> QObject *id; >> JSONMessageParser parser; >> @@ -176,7 +194,8 @@ struct Monitor { >> int print_calls_nr; >> #endif >> QError *error; >> - QLIST_HEAD(,mon_fd_t) fds; >> + QLIST_HEAD(, mon_fd_t) fds; >> + QLIST_HEAD(, mon_fdset_t) fdsets; >> QLIST_ENTRY(Monitor) entry; >> }; >> >> @@ -2389,6 +2408,157 @@ int monitor_get_fd(Monitor *mon, const char *fdname) >> return -1; >> } >> >> +static void monitor_fdset_cleanup(mon_fdset_t *mon_fdset) >> +{ >> + mon_fdset_fd_t *mon_fdset_fd; >> + mon_fdset_fd_t *mon_fdset_fd_next; >> + >> + if (mon_fdset->refcount != 0) { >> + return; >> + } >> + >> + QLIST_FOREACH_SAFE(mon_fdset_fd, &mon_fdset->fds, next, mon_fdset_fd_next) { >> + if (!mon_fdset->in_use || mon_fdset_fd->removed) { >> + close(mon_fdset_fd->fd); >> + QLIST_REMOVE(mon_fdset_fd, next); >> + g_free(mon_fdset_fd); >> + } >> + } >> + >> + if (QLIST_EMPTY(&mon_fdset->fds)) { >> + QLIST_REMOVE(mon_fdset, next); >> + g_free(mon_fdset); >> + } >> +} >> + >> +AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, Error **errp) >> +{ >> + int fd; >> + Monitor *mon = cur_mon; >> + mon_fdset_t *mon_fdset; >> + mon_fdset_fd_t *mon_fdset_fd; >> + AddfdInfo *fdinfo; >> + >> + fd = qemu_chr_fe_get_msgfd(mon->chr); >> + if (fd == -1) { >> + qerror_report(QERR_FD_NOT_SUPPLIED); >> + return NULL; >> + } >> + >> + if (has_fdset_id) { >> + QLIST_FOREACH(mon_fdset, &mon->fdsets, next) { >> + if (mon_fdset->id == fdset_id) { >> + break; >> + } >> + } >> + if (mon_fdset == NULL) { >> + qerror_report(QERR_FDSET_NOT_FOUND, fdset_id); >> + return NULL; > > fd is leaked? > Yes, it looks like it is. I'll fix that. >> + } >> + } else { >> + int64_t fdset_id_prev = -1; >> + mon_fdset_t *mon_fdset_cur = QLIST_FIRST(&mon->fdsets); >> + >> + /* Use first available fdset ID */ >> + QLIST_FOREACH(mon_fdset, &mon->fdsets, next) { >> + mon_fdset_cur = mon_fdset; >> + if (fdset_id_prev == mon_fdset_cur->id - 1) { >> + fdset_id_prev = mon_fdset_cur->id; >> + continue; >> + } >> + break; >> + } >> + >> + mon_fdset = g_malloc0(sizeof(*mon_fdset)); >> + mon_fdset->id = fdset_id_prev + 1; >> + mon_fdset->refcount = 0; >> + mon_fdset->in_use = true; >> + >> + /* The fdset list is ordered by fdset ID */ >> + if (mon_fdset->id == 0) { >> + QLIST_INSERT_HEAD(&mon->fdsets, mon_fdset, next); >> + } else if (mon_fdset->id < mon_fdset_cur->id) { >> + QLIST_INSERT_BEFORE(mon_fdset_cur, mon_fdset, next); >> + } else { >> + QLIST_INSERT_AFTER(mon_fdset_cur, mon_fdset, next); >> + } >> + } >> + >> + mon_fdset_fd = g_malloc0(sizeof(*mon_fdset_fd)); >> + mon_fdset_fd->fd = fd; >> + mon_fdset_fd->removed = false; >> + QLIST_INSERT_HEAD(&mon_fdset->fds, mon_fdset_fd, next); >> + >> + fdinfo = g_malloc0(sizeof(*fdinfo)); >> + fdinfo->fdset_id = mon_fdset->id; >> + fdinfo->fd = mon_fdset_fd->fd; >> + >> + return fdinfo; >> +} >> + >> +void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp) >> +{ >> + Monitor *mon = cur_mon; >> + mon_fdset_t *mon_fdset; >> + mon_fdset_fd_t *mon_fdset_fd; >> + char fd_str[20]; >> + >> + QLIST_FOREACH(mon_fdset, &mon->fdsets, next) { >> + if (mon_fdset->id != fdset_id) { >> + continue; >> + } >> + QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) { >> + if (has_fd && mon_fdset_fd->fd != fd) { >> + continue; >> + } >> + mon_fdset_fd->removed = true; >> + if (has_fd) { >> + break; >> + } >> + } >> + monitor_fdset_cleanup(mon_fdset); >> + return; >> + } >> + snprintf(fd_str, sizeof(fd_str), "%ld", fd); >> + qerror_report(QERR_FD_NOT_FOUND, fd_str); > > Why use an fd_str instead of passing an int64_t into the error > message? This also assumed sizeof(long) == 8, which isn't true on > 32-bit hosts, so %ld should be %"PRId64". Can I pass an int64_t into the message if it takes a string? I thought int64_t was a long long in 32-bit mode, but perhaps that's not always the case? > > There is a new policy on error reporting. I think this patch series > may be affected/conflict, please qemu-devel to check. I think Luiz > can also help here. Ok thanks, I'll take a look at qemu-devel.
On Tue, Aug 7, 2012 at 8:59 PM, Corey Bryant <coreyb@linux.vnet.ibm.com> wrote: > > > On 08/07/2012 02:16 PM, Stefan Hajnoczi wrote: >> >> On Fri, Aug 3, 2012 at 6:28 PM, Corey Bryant <coreyb@linux.vnet.ibm.com> >> wrote: >>> + snprintf(fd_str, sizeof(fd_str), "%ld", fd); >>> + qerror_report(QERR_FD_NOT_FOUND, fd_str); >> >> >> Why use an fd_str instead of passing an int64_t into the error >> message? This also assumed sizeof(long) == 8, which isn't true on >> 32-bit hosts, so %ld should be %"PRId64". > > > Can I pass an int64_t into the message if it takes a string? > > I thought int64_t was a long long in 32-bit mode, but perhaps that's not > always the case? The PRId64 format specifier macro from the C standard hides this so you can pass int64_t values to printf()-style functions in a portable way. Stefan
On 08/08/2012 04:52 AM, Stefan Hajnoczi wrote: > On Tue, Aug 7, 2012 at 8:59 PM, Corey Bryant <coreyb@linux.vnet.ibm.com> wrote: >> >> >> On 08/07/2012 02:16 PM, Stefan Hajnoczi wrote: >>> >>> On Fri, Aug 3, 2012 at 6:28 PM, Corey Bryant <coreyb@linux.vnet.ibm.com> >>> wrote: >>>> + snprintf(fd_str, sizeof(fd_str), "%ld", fd); >>>> + qerror_report(QERR_FD_NOT_FOUND, fd_str); >>> >>> >>> Why use an fd_str instead of passing an int64_t into the error >>> message? This also assumed sizeof(long) == 8, which isn't true on >>> 32-bit hosts, so %ld should be %"PRId64". >> >> >> Can I pass an int64_t into the message if it takes a string? >> >> I thought int64_t was a long long in 32-bit mode, but perhaps that's not >> always the case? > > The PRId64 format specifier macro from the C standard hides this so > you can pass int64_t values to printf()-style functions in a portable > way. > > Stefan > Thanks, I'll use PRId64.
diff --git a/monitor.c b/monitor.c index 49dccfe..9aa9f7e 100644 --- a/monitor.c +++ b/monitor.c @@ -140,6 +140,24 @@ struct mon_fd_t { QLIST_ENTRY(mon_fd_t) next; }; +/* file descriptor associated with a file descriptor set */ +typedef struct mon_fdset_fd_t mon_fdset_fd_t; +struct mon_fdset_fd_t { + int fd; + bool removed; + QLIST_ENTRY(mon_fdset_fd_t) next; +}; + +/* file descriptor set containing fds passed via SCM_RIGHTS */ +typedef struct mon_fdset_t mon_fdset_t; +struct mon_fdset_t { + int64_t id; + int refcount; + bool in_use; + QLIST_HEAD(, mon_fdset_fd_t) fds; + QLIST_ENTRY(mon_fdset_t) next; +}; + typedef struct MonitorControl { QObject *id; JSONMessageParser parser; @@ -176,7 +194,8 @@ struct Monitor { int print_calls_nr; #endif QError *error; - QLIST_HEAD(,mon_fd_t) fds; + QLIST_HEAD(, mon_fd_t) fds; + QLIST_HEAD(, mon_fdset_t) fdsets; QLIST_ENTRY(Monitor) entry; }; @@ -2389,6 +2408,157 @@ int monitor_get_fd(Monitor *mon, const char *fdname) return -1; } +static void monitor_fdset_cleanup(mon_fdset_t *mon_fdset) +{ + mon_fdset_fd_t *mon_fdset_fd; + mon_fdset_fd_t *mon_fdset_fd_next; + + if (mon_fdset->refcount != 0) { + return; + } + + QLIST_FOREACH_SAFE(mon_fdset_fd, &mon_fdset->fds, next, mon_fdset_fd_next) { + if (!mon_fdset->in_use || mon_fdset_fd->removed) { + close(mon_fdset_fd->fd); + QLIST_REMOVE(mon_fdset_fd, next); + g_free(mon_fdset_fd); + } + } + + if (QLIST_EMPTY(&mon_fdset->fds)) { + QLIST_REMOVE(mon_fdset, next); + g_free(mon_fdset); + } +} + +AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, Error **errp) +{ + int fd; + Monitor *mon = cur_mon; + mon_fdset_t *mon_fdset; + mon_fdset_fd_t *mon_fdset_fd; + AddfdInfo *fdinfo; + + fd = qemu_chr_fe_get_msgfd(mon->chr); + if (fd == -1) { + qerror_report(QERR_FD_NOT_SUPPLIED); + return NULL; + } + + if (has_fdset_id) { + QLIST_FOREACH(mon_fdset, &mon->fdsets, next) { + if (mon_fdset->id == fdset_id) { + break; + } + } + if (mon_fdset == NULL) { + qerror_report(QERR_FDSET_NOT_FOUND, fdset_id); + return NULL; + } + } else { + int64_t fdset_id_prev = -1; + mon_fdset_t *mon_fdset_cur = QLIST_FIRST(&mon->fdsets); + + /* Use first available fdset ID */ + QLIST_FOREACH(mon_fdset, &mon->fdsets, next) { + mon_fdset_cur = mon_fdset; + if (fdset_id_prev == mon_fdset_cur->id - 1) { + fdset_id_prev = mon_fdset_cur->id; + continue; + } + break; + } + + mon_fdset = g_malloc0(sizeof(*mon_fdset)); + mon_fdset->id = fdset_id_prev + 1; + mon_fdset->refcount = 0; + mon_fdset->in_use = true; + + /* The fdset list is ordered by fdset ID */ + if (mon_fdset->id == 0) { + QLIST_INSERT_HEAD(&mon->fdsets, mon_fdset, next); + } else if (mon_fdset->id < mon_fdset_cur->id) { + QLIST_INSERT_BEFORE(mon_fdset_cur, mon_fdset, next); + } else { + QLIST_INSERT_AFTER(mon_fdset_cur, mon_fdset, next); + } + } + + mon_fdset_fd = g_malloc0(sizeof(*mon_fdset_fd)); + mon_fdset_fd->fd = fd; + mon_fdset_fd->removed = false; + QLIST_INSERT_HEAD(&mon_fdset->fds, mon_fdset_fd, next); + + fdinfo = g_malloc0(sizeof(*fdinfo)); + fdinfo->fdset_id = mon_fdset->id; + fdinfo->fd = mon_fdset_fd->fd; + + return fdinfo; +} + +void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp) +{ + Monitor *mon = cur_mon; + mon_fdset_t *mon_fdset; + mon_fdset_fd_t *mon_fdset_fd; + char fd_str[20]; + + QLIST_FOREACH(mon_fdset, &mon->fdsets, next) { + if (mon_fdset->id != fdset_id) { + continue; + } + QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) { + if (has_fd && mon_fdset_fd->fd != fd) { + continue; + } + mon_fdset_fd->removed = true; + if (has_fd) { + break; + } + } + monitor_fdset_cleanup(mon_fdset); + return; + } + snprintf(fd_str, sizeof(fd_str), "%ld", fd); + qerror_report(QERR_FD_NOT_FOUND, fd_str); +} + +FdsetInfoList *qmp_query_fdsets(Error **errp) +{ + Monitor *mon = cur_mon; + mon_fdset_t *mon_fdset; + mon_fdset_fd_t *mon_fdset_fd; + FdsetInfoList *fdset_list = NULL; + + QLIST_FOREACH(mon_fdset, &mon->fdsets, next) { + FdsetInfoList *fdset_info = g_malloc0(sizeof(*fdset_info)); + FdsetFdInfoList *fdsetfd_list = NULL; + + fdset_info->value = g_malloc0(sizeof(*fdset_info->value)); + fdset_info->value->fdset_id = mon_fdset->id; + fdset_info->value->refcount = mon_fdset->refcount; + fdset_info->value->in_use = mon_fdset->in_use; + + QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) { + FdsetFdInfoList *fdsetfd_info = g_malloc0(sizeof(*fdsetfd_info)); + + fdsetfd_info->value = g_malloc0(sizeof(*fdsetfd_info->value)); + fdsetfd_info->value->fd = mon_fdset_fd->fd; + fdsetfd_info->value->removed = mon_fdset_fd->removed; + + fdsetfd_info->next = fdsetfd_list; + fdsetfd_list = fdsetfd_info; + } + + fdset_info->value->fds = fdsetfd_list; + + fdset_info->next = fdset_list; + fdset_list = fdset_info; + } + + return fdset_list; +} + /* mon_cmds and info_cmds would be sorted at runtime */ static mon_cmd_t mon_cmds[] = { #include "hmp-commands.h" diff --git a/qapi-schema.json b/qapi-schema.json index bc55ed2..4d46e5b 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -2183,3 +2183,106 @@ # Since: 0.14.0 ## { 'command': 'closefd', 'data': {'fdname': 'str'} } + +# @AddfdInfo: +# +# Information about a file descriptor that was added to an fd set. +# +# @fdset_id: The ID of the fd set that @fd was added to. +# +# @fd: The file descriptor that was received via SCM rights and +# added to the fd set. +# +# Since: 1.2.0 +## +{ 'type': 'AddfdInfo', 'data': {'fdset_id': 'int', 'fd': 'int'} } + +## +# @add-fd: +# +# Add a file descriptor, that was passed via SCM rights, to an fd set. +# +# @fdset_id: #optional The ID of the fd set to add the file descriptor to. +# +# Returns: @AddfdInfo on success +# If file descriptor was not received, FdNotSupplied +# If @fdset_id does not exist, FdSetNotFound +# +# Note: If @fdset_id is not specified, a new fd set will be created. +# +# Since: 1.2.0 +## +{ 'command': 'add-fd', 'data': {'*fdset_id': 'int'}, + 'returns': 'AddfdInfo' } + +## +# @remove-fd: +# +# Remove a file descriptor from an fd set. +# +# @fdset_id: The ID of the fd set that the file descriptor belongs to. +# +# @fd: #optional The file descriptor that is to be removed. +# +# Returns: Nothing on success +# If @fdset_id or @fd is not found, FdNotFound +# +# Since: 1.2.0 +# +# Notes: File descriptors that are removed: +# o will not be closed until the reference count corresponding +# to @fdset_id reaches zero. +# o will not be available for use after successful completion +# of the remove-fd command. +# +# If @fd is not specified, all file descriptors in @fdset_id +# will be removed. +## +{ 'command': 'remove-fd', 'data': {'fdset_id': 'int', '*fd': 'int'} } + +## +# @FdsetFdInfo: +# +# Information about a file descriptor that belongs to an fd set. +# +# @fd: The file descriptor value. +# +# @removed: If true, the remove-fd command has been issued for this fd. +# +# Since: 1.2.0 +## +{ 'type': 'FdsetFdInfo', 'data': {'fd': 'int', 'removed': 'bool'} } + +## +# @FdsetInfo: +# +# Information about an fd set. +# +# @fdset_id: The ID of the fd set. +# +# @refcount: A count of the number of outstanding dup() references to +# this fd set. +# +# @in_use: If true, this fd set is in use by a connected QMP monitor. +# +# @fds: A list of file descriptors that belong to this fd set. +# +# Since: 1.2.0 +## +{ 'type': 'FdsetInfo', + 'data': {'fdset_id': 'int', 'refcount': 'int', 'in_use': 'bool', + 'fds': ['FdsetFdInfo']} } + +## +# @query-fdsets: +# +# Return information describing all fd sets. +# +# Returns: A list of @FdsetInfo +# +# Since: 1.2.0 +# +# Notes: File descriptors are not closed until @refcount is zero, +# and either @in_use is false or @removed is true. +## +{ 'command': 'query-fdsets', 'returns': ['FdsetInfo'] } diff --git a/qerror.c b/qerror.c index 92c4eff..63a0aa1 100644 --- a/qerror.c +++ b/qerror.c @@ -148,6 +148,10 @@ static const QErrorStringTable qerror_table[] = { .desc = "No file descriptor supplied via SCM_RIGHTS", }, { + .error_fmt = QERR_FDSET_NOT_FOUND, + .desc = "File descriptor set with ID '%(id)' not found", + }, + { .error_fmt = QERR_FEATURE_DISABLED, .desc = "The feature '%(name)' is not enabled", }, diff --git a/qerror.h b/qerror.h index b4c8758..b908d3f 100644 --- a/qerror.h +++ b/qerror.h @@ -133,6 +133,9 @@ QError *qobject_to_qerror(const QObject *obj); #define QERR_FD_NOT_SUPPLIED \ "{ 'class': 'FdNotSupplied', 'data': {} }" +#define QERR_FDSET_NOT_FOUND \ + "{ 'class': 'FdSetNotFound', 'data': { 'id': %ld } }" + #define QERR_FEATURE_DISABLED \ "{ 'class': 'FeatureDisabled', 'data': { 'name': %s } }" diff --git a/qmp-commands.hx b/qmp-commands.hx index e3cf3c5..7141168 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -926,6 +926,132 @@ Example: EQMP + { + .name = "add-fd", + .args_type = "fdset_id:i?", + .params = "add-fd fdset_id", + .help = "Add a file descriptor, that was passed via SCM rights, to an fd set", + .mhandler.cmd_new = qmp_marshal_input_add_fd, + }, + +SQMP +add-fd +------- + +Add a file descriptor, that was passed via SCM rights, to an fd set. + +Arguments: + +- "fdset_id": The ID of the fd set to add the file descriptor to. + (json-int, optional) + +Return a json-object with the following information: + +- "fdset_id": The ID of the fd set that the fd was added to. (json-int) +- "fd": The file descriptor that was received via SCM rights and added to the + fd set. (json-int) + +Example: + +-> { "execute": "add-fd", "arguments": { "fdset_id": 1 } } +<- { "return": { "fdset_id": 1, "fd": 3 } } + +Note: If "fdset_id" is not specified, a new fd set will be created. + +EQMP + + { + .name = "remove-fd", + .args_type = "fdset_id:i,fd:i?", + .params = "remove-fd fdset_id fd", + .help = "Remove a file descriptor from an fd set", + .mhandler.cmd_new = qmp_marshal_input_remove_fd, + }, + +SQMP +remove-fd +--------- + +Remove a file descriptor from an fd set. + +Arguments: + +- "fdset_id": The ID of the fd set that the file descriptor belongs to. + (json-int) +- "fd": The file descriptor that is to be removed. (json-int, optional) + +Example: + +-> { "execute": "remove-fd", "arguments": { "fdset_id": 1, "fd": 3 } } +<- { "return": {} } + +Notes: + +(1) File descriptors that are removed: + (a) will not be closed until the reference count corresponding to fdset_id + reaches zero. + (b) will not be available for use after successful completion of the + remove-fd command. +(2) If "fd" is not specified, all file descriptors in "fdset_id" will be + removed. + +EQMP + + { + .name = "query-fdsets", + .args_type = "", + .help = "Return information describing all fd sets", + .mhandler.cmd_new = qmp_marshal_input_query_fdsets, + }, + +SQMP +query-fdsets +------------- + +Return information describing all fd sets. + +Arguments: None + +Example: + +-> { "execute": "query-fdsets" } +<- { "return": [ + { + "fdset_id": 1 + "refcount": 0, + "in_use": true, + "fds": [ + { + "fd": 21, + "removed": false + }, + { + "fd": 23, + "removed": false + } + ], + }, + { + "fdset_id": 2 + "refcount": 0, + "in_use": true, + "fds": [ + { + "fd": 22, + "removed": false + } + ], + } + ] + } + +Notes: + +(1) File descriptors are not closed until refcount is zero, and + either in_use is false or removed is true. + +EQMP + { .name = "block_passwd", .args_type = "device:B,password:s",
This patch adds support that enables passing of file descriptors to the QEMU monitor where they will be stored in specified file descriptor sets. A file descriptor set can be used by a client like libvirt to store file descriptors for the same file. This allows the client to open a file with different access modes (O_RDWR, O_WRONLY, O_RDONLY) and add/remove the passed fds to/from an fd set as needed. This will allow QEMU to (in a later patch in this series) "open" and "reopen" the same file by dup()ing the fd in the fd set that corresponds to the file, where the fd has the matching access mode flag that QEMU requests. The new QMP commands are: add-fd: Add a file descriptor to an fd set remove-fd: Remove a file descriptor from an fd set query-fdsets: Return information describing all fd sets Note: These commands are not compatible with the existing getfd and closefd QMP commands. v5: -This patch is new in v5 and replaces the pass-fd QMP command from v4. -By grouping fds in fd sets, we ease managability with an fd set per file, addressing concerns raised in v4 about handling "reopens" and preventing fd leakage. (eblake@redhat.com, kwolf@redhat.com, dberrange@redhat.com) v6 -Make @fd optional for remove-fd (eblake@redhat.com) -Make @fdset-id optional for add-fd (eblake@redhat.com) Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com> --- monitor.c | 172 +++++++++++++++++++++++++++++++++++++++++++++++++++++- qapi-schema.json | 103 ++++++++++++++++++++++++++++++++ qerror.c | 4 ++ qerror.h | 3 + qmp-commands.hx | 126 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 407 insertions(+), 1 deletion(-)