* [Qemu-devel] [PATCH v2 1/4] qapi: Convert getfd and closefd
2012-06-08 15:42 [Qemu-devel] [PATCH v2 0/4] file descriptor passing using passfd Corey Bryant
@ 2012-06-08 15:42 ` Corey Bryant
2012-06-13 19:41 ` Luiz Capitulino
2012-06-13 19:42 ` Luiz Capitulino
2012-06-08 15:42 ` [Qemu-devel] [PATCH v2 2/4] qapi: Add passfd QMP command Corey Bryant
` (4 subsequent siblings)
5 siblings, 2 replies; 24+ messages in thread
From: Corey Bryant @ 2012-06-08 15:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, aliguori, stefanha, libvir-list, Corey Bryant, eblake
v2:
- Convert getfd and closefd to QAPI (lcapitulino@redhat.com)
- Remove changes that returned fd from getfd (lcapitulino@redhat.com)
- Wrap hmp_* functions around qmp_* functions (kwolf@redhat.com)
- Move hmp_* functions to hmp.c (lcapitulino@redhat.com)
- Drop .user_print lines (lcapitulino@redhat.com)
- Use 'cmd' instead of 'cmd_new' for HMP (lcapitulino@redhat.com)
- Change QMP command existance back to 0.14 (lcapitulino@redhat.com)
Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
---
hmp-commands.hx | 6 ++----
hmp.c | 18 ++++++++++++++++++
hmp.h | 2 ++
monitor.c | 34 ++++++++++++++++------------------
qapi-schema.json | 29 +++++++++++++++++++++++++++++
qmp-commands.hx | 6 ++----
6 files changed, 69 insertions(+), 26 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index f5d9d91..eea8b32 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1236,8 +1236,7 @@ ETEXI
.args_type = "fdname:s",
.params = "getfd name",
.help = "receive a file descriptor via SCM rights and assign it a name",
- .user_print = monitor_user_noop,
- .mhandler.cmd_new = do_getfd,
+ .mhandler.cmd = hmp_getfd,
},
STEXI
@@ -1253,8 +1252,7 @@ ETEXI
.args_type = "fdname:s",
.params = "closefd name",
.help = "close a file descriptor previously passed via SCM rights",
- .user_print = monitor_user_noop,
- .mhandler.cmd_new = do_closefd,
+ .mhandler.cmd = hmp_closefd,
},
STEXI
diff --git a/hmp.c b/hmp.c
index 2ce8cb9..6241856 100644
--- a/hmp.c
+++ b/hmp.c
@@ -999,3 +999,21 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
qmp_netdev_del(id, &err);
hmp_handle_error(mon, &err);
}
+
+void hmp_getfd(Monitor *mon, const QDict *qdict)
+{
+ const char *fdname = qdict_get_str(qdict, "fdname");
+ Error *errp = NULL;
+
+ qmp_getfd(fdname, &errp);
+ hmp_handle_error(mon, &errp);
+}
+
+void hmp_closefd(Monitor *mon, const QDict *qdict)
+{
+ const char *fdname = qdict_get_str(qdict, "fdname");
+ Error *errp = NULL;
+
+ qmp_closefd(fdname, &errp);
+ hmp_handle_error(mon, &errp);
+}
diff --git a/hmp.h b/hmp.h
index 79d138d..8d2b0d7 100644
--- a/hmp.h
+++ b/hmp.h
@@ -64,5 +64,7 @@ void hmp_device_del(Monitor *mon, const QDict *qdict);
void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
void hmp_netdev_add(Monitor *mon, const QDict *qdict);
void hmp_netdev_del(Monitor *mon, const QDict *qdict);
+void hmp_getfd(Monitor *mon, const QDict *qdict);
+void hmp_closefd(Monitor *mon, const QDict *qdict);
#endif
diff --git a/monitor.c b/monitor.c
index a3bc2c7..4c53cb6 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2182,48 +2182,46 @@ static void do_inject_mce(Monitor *mon, const QDict *qdict)
}
#endif
-static int do_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
+void qmp_getfd(const char *fdname, Error **errp)
{
- const char *fdname = qdict_get_str(qdict, "fdname");
mon_fd_t *monfd;
int fd;
- fd = qemu_chr_fe_get_msgfd(mon->chr);
+ fd = qemu_chr_fe_get_msgfd(cur_mon->chr);
if (fd == -1) {
- qerror_report(QERR_FD_NOT_SUPPLIED);
- return -1;
+ error_set(errp, QERR_FD_NOT_SUPPLIED);
+ return;
}
if (qemu_isdigit(fdname[0])) {
- qerror_report(QERR_INVALID_PARAMETER_VALUE, "fdname",
- "a name not starting with a digit");
- return -1;
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdname",
+ "a name not starting with a digit");
+ return;
}
- QLIST_FOREACH(monfd, &mon->fds, next) {
+ QLIST_FOREACH(monfd, &cur_mon->fds, next) {
if (strcmp(monfd->name, fdname) != 0) {
continue;
}
close(monfd->fd);
monfd->fd = fd;
- return 0;
+ return;
}
monfd = g_malloc0(sizeof(mon_fd_t));
monfd->name = g_strdup(fdname);
monfd->fd = fd;
- QLIST_INSERT_HEAD(&mon->fds, monfd, next);
- return 0;
+ QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
+ return;
}
-static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
+void qmp_closefd(const char *fdname, Error **errp)
{
- const char *fdname = qdict_get_str(qdict, "fdname");
mon_fd_t *monfd;
- QLIST_FOREACH(monfd, &mon->fds, next) {
+ QLIST_FOREACH(monfd, &cur_mon->fds, next) {
if (strcmp(monfd->name, fdname) != 0) {
continue;
}
@@ -2232,11 +2230,11 @@ static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
close(monfd->fd);
g_free(monfd->name);
g_free(monfd);
- return 0;
+ return;
}
- qerror_report(QERR_FD_NOT_FOUND, fdname);
- return -1;
+ error_set(errp, QERR_FD_NOT_FOUND, fdname);
+ return;
}
static void do_loadvm(Monitor *mon, const QDict *qdict)
diff --git a/qapi-schema.json b/qapi-schema.json
index 3b6e346..6be1d90 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1862,3 +1862,32 @@
# Since: 0.14.0
##
{ 'command': 'netdev_del', 'data': {'id': 'str'} }
+
+##
+# @getfd:
+#
+# Receive a file descriptor via SCM rights and assign it a name
+#
+# @fdname: file descriptor name
+#
+# Returns: Nothing on success
+# If file descriptor was not received, FdNotSupplied
+# If @fdname is not valid, InvalidParameterType
+#
+# Since: 0.14.0
+##
+{ 'command': 'getfd', 'data': {'fdname': 'str'} }
+
+##
+# @closefd:
+#
+# Close a file descriptor previously passed via SCM rights
+#
+# @fdname: file descriptor name
+#
+# Returns: Nothing on success
+# If @fdname is not found, FdNotFound
+#
+# Since: 0.14.0
+##
+{ 'command': 'closefd', 'data': {'fdname': 'str'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 2e1a38e..f8c0f68 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -873,8 +873,7 @@ EQMP
.args_type = "fdname:s",
.params = "getfd name",
.help = "receive a file descriptor via SCM rights and assign it a name",
- .user_print = monitor_user_noop,
- .mhandler.cmd_new = do_getfd,
+ .mhandler.cmd_new = qmp_marshal_input_getfd,
},
SQMP
@@ -899,8 +898,7 @@ EQMP
.args_type = "fdname:s",
.params = "closefd name",
.help = "close a file descriptor previously passed via SCM rights",
- .user_print = monitor_user_noop,
- .mhandler.cmd_new = do_closefd,
+ .mhandler.cmd_new = qmp_marshal_input_closefd,
},
SQMP
--
1.7.10.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH v2 1/4] qapi: Convert getfd and closefd
2012-06-08 15:42 ` [Qemu-devel] [PATCH v2 1/4] qapi: Convert getfd and closefd Corey Bryant
@ 2012-06-13 19:41 ` Luiz Capitulino
2012-06-13 20:10 ` Corey Bryant
2012-06-13 19:42 ` Luiz Capitulino
1 sibling, 1 reply; 24+ messages in thread
From: Luiz Capitulino @ 2012-06-13 19:41 UTC (permalink / raw)
To: Corey Bryant; +Cc: kwolf, aliguori, stefanha, libvir-list, qemu-devel, eblake
On Fri, 8 Jun 2012 11:42:56 -0400
Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
> v2:
> - Convert getfd and closefd to QAPI (lcapitulino@redhat.com)
> - Remove changes that returned fd from getfd (lcapitulino@redhat.com)
> - Wrap hmp_* functions around qmp_* functions (kwolf@redhat.com)
> - Move hmp_* functions to hmp.c (lcapitulino@redhat.com)
> - Drop .user_print lines (lcapitulino@redhat.com)
> - Use 'cmd' instead of 'cmd_new' for HMP (lcapitulino@redhat.com)
> - Change QMP command existance back to 0.14 (lcapitulino@redhat.com)
>
> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
> ---
> hmp-commands.hx | 6 ++----
> hmp.c | 18 ++++++++++++++++++
> hmp.h | 2 ++
> monitor.c | 34 ++++++++++++++++------------------
> qapi-schema.json | 29 +++++++++++++++++++++++++++++
> qmp-commands.hx | 6 ++----
> 6 files changed, 69 insertions(+), 26 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index f5d9d91..eea8b32 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1236,8 +1236,7 @@ ETEXI
> .args_type = "fdname:s",
> .params = "getfd name",
> .help = "receive a file descriptor via SCM rights and assign it a name",
> - .user_print = monitor_user_noop,
> - .mhandler.cmd_new = do_getfd,
> + .mhandler.cmd = hmp_getfd,
> },
>
> STEXI
> @@ -1253,8 +1252,7 @@ ETEXI
> .args_type = "fdname:s",
> .params = "closefd name",
> .help = "close a file descriptor previously passed via SCM rights",
> - .user_print = monitor_user_noop,
> - .mhandler.cmd_new = do_closefd,
> + .mhandler.cmd = hmp_closefd,
> },
>
> STEXI
> diff --git a/hmp.c b/hmp.c
> index 2ce8cb9..6241856 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -999,3 +999,21 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
> qmp_netdev_del(id, &err);
> hmp_handle_error(mon, &err);
> }
> +
> +void hmp_getfd(Monitor *mon, const QDict *qdict)
> +{
> + const char *fdname = qdict_get_str(qdict, "fdname");
> + Error *errp = NULL;
> +
> + qmp_getfd(fdname, &errp);
> + hmp_handle_error(mon, &errp);
> +}
> +
> +void hmp_closefd(Monitor *mon, const QDict *qdict)
> +{
> + const char *fdname = qdict_get_str(qdict, "fdname");
> + Error *errp = NULL;
> +
> + qmp_closefd(fdname, &errp);
> + hmp_handle_error(mon, &errp);
> +}
> diff --git a/hmp.h b/hmp.h
> index 79d138d..8d2b0d7 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -64,5 +64,7 @@ void hmp_device_del(Monitor *mon, const QDict *qdict);
> void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
> void hmp_netdev_add(Monitor *mon, const QDict *qdict);
> void hmp_netdev_del(Monitor *mon, const QDict *qdict);
> +void hmp_getfd(Monitor *mon, const QDict *qdict);
> +void hmp_closefd(Monitor *mon, const QDict *qdict);
>
> #endif
> diff --git a/monitor.c b/monitor.c
> index a3bc2c7..4c53cb6 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2182,48 +2182,46 @@ static void do_inject_mce(Monitor *mon, const QDict *qdict)
> }
> #endif
>
> -static int do_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
> +void qmp_getfd(const char *fdname, Error **errp)
> {
> - const char *fdname = qdict_get_str(qdict, "fdname");
> mon_fd_t *monfd;
> int fd;
>
> - fd = qemu_chr_fe_get_msgfd(mon->chr);
> + fd = qemu_chr_fe_get_msgfd(cur_mon->chr);
> if (fd == -1) {
> - qerror_report(QERR_FD_NOT_SUPPLIED);
> - return -1;
> + error_set(errp, QERR_FD_NOT_SUPPLIED);
> + return;
> }
>
> if (qemu_isdigit(fdname[0])) {
> - qerror_report(QERR_INVALID_PARAMETER_VALUE, "fdname",
> - "a name not starting with a digit");
> - return -1;
> + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdname",
> + "a name not starting with a digit");
> + return;
> }
>
> - QLIST_FOREACH(monfd, &mon->fds, next) {
> + QLIST_FOREACH(monfd, &cur_mon->fds, next) {
> if (strcmp(monfd->name, fdname) != 0) {
> continue;
> }
>
> close(monfd->fd);
> monfd->fd = fd;
> - return 0;
> + return;
> }
>
> monfd = g_malloc0(sizeof(mon_fd_t));
> monfd->name = g_strdup(fdname);
> monfd->fd = fd;
>
> - QLIST_INSERT_HEAD(&mon->fds, monfd, next);
> - return 0;
> + QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
> + return;
You have a few of this kind of gratuitous return, please drop 'em.
> }
>
> -static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
> +void qmp_closefd(const char *fdname, Error **errp)
> {
> - const char *fdname = qdict_get_str(qdict, "fdname");
> mon_fd_t *monfd;
>
> - QLIST_FOREACH(monfd, &mon->fds, next) {
> + QLIST_FOREACH(monfd, &cur_mon->fds, next) {
> if (strcmp(monfd->name, fdname) != 0) {
> continue;
> }
> @@ -2232,11 +2230,11 @@ static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
> close(monfd->fd);
> g_free(monfd->name);
> g_free(monfd);
> - return 0;
> + return;
> }
>
> - qerror_report(QERR_FD_NOT_FOUND, fdname);
> - return -1;
> + error_set(errp, QERR_FD_NOT_FOUND, fdname);
> + return;
> }
>
> static void do_loadvm(Monitor *mon, const QDict *qdict)
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 3b6e346..6be1d90 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1862,3 +1862,32 @@
> # Since: 0.14.0
> ##
> { 'command': 'netdev_del', 'data': {'id': 'str'} }
> +
> +##
> +# @getfd:
> +#
> +# Receive a file descriptor via SCM rights and assign it a name
> +#
> +# @fdname: file descriptor name
> +#
> +# Returns: Nothing on success
> +# If file descriptor was not received, FdNotSupplied
> +# If @fdname is not valid, InvalidParameterType
Please, add a note explaining that getfd automatically closes fds if
fdname matches an existing name.
Otherwise this conversion looks very good to me.
> +#
> +# Since: 0.14.0
> +##
> +{ 'command': 'getfd', 'data': {'fdname': 'str'} }
> +
> +##
> +# @closefd:
> +#
> +# Close a file descriptor previously passed via SCM rights
> +#
> +# @fdname: file descriptor name
> +#
> +# Returns: Nothing on success
> +# If @fdname is not found, FdNotFound
> +#
> +# Since: 0.14.0
> +##
> +{ 'command': 'closefd', 'data': {'fdname': 'str'} }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 2e1a38e..f8c0f68 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -873,8 +873,7 @@ EQMP
> .args_type = "fdname:s",
> .params = "getfd name",
> .help = "receive a file descriptor via SCM rights and assign it a name",
> - .user_print = monitor_user_noop,
> - .mhandler.cmd_new = do_getfd,
> + .mhandler.cmd_new = qmp_marshal_input_getfd,
> },
>
> SQMP
> @@ -899,8 +898,7 @@ EQMP
> .args_type = "fdname:s",
> .params = "closefd name",
> .help = "close a file descriptor previously passed via SCM rights",
> - .user_print = monitor_user_noop,
> - .mhandler.cmd_new = do_closefd,
> + .mhandler.cmd_new = qmp_marshal_input_closefd,
> },
>
> SQMP
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH v2 1/4] qapi: Convert getfd and closefd
2012-06-13 19:41 ` Luiz Capitulino
@ 2012-06-13 20:10 ` Corey Bryant
0 siblings, 0 replies; 24+ messages in thread
From: Corey Bryant @ 2012-06-13 20:10 UTC (permalink / raw)
To: Luiz Capitulino
Cc: kwolf, aliguori, stefanha, libvir-list, qemu-devel, eblake
On 06/13/2012 03:41 PM, Luiz Capitulino wrote:
> On Fri, 8 Jun 2012 11:42:56 -0400
> Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
>
>> v2:
>> - Convert getfd and closefd to QAPI (lcapitulino@redhat.com)
>> - Remove changes that returned fd from getfd (lcapitulino@redhat.com)
>> - Wrap hmp_* functions around qmp_* functions (kwolf@redhat.com)
>> - Move hmp_* functions to hmp.c (lcapitulino@redhat.com)
>> - Drop .user_print lines (lcapitulino@redhat.com)
>> - Use 'cmd' instead of 'cmd_new' for HMP (lcapitulino@redhat.com)
>> - Change QMP command existance back to 0.14 (lcapitulino@redhat.com)
>>
>> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
>> ---
>> hmp-commands.hx | 6 ++----
>> hmp.c | 18 ++++++++++++++++++
>> hmp.h | 2 ++
>> monitor.c | 34 ++++++++++++++++------------------
>> qapi-schema.json | 29 +++++++++++++++++++++++++++++
>> qmp-commands.hx | 6 ++----
>> 6 files changed, 69 insertions(+), 26 deletions(-)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index f5d9d91..eea8b32 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -1236,8 +1236,7 @@ ETEXI
>> .args_type = "fdname:s",
>> .params = "getfd name",
>> .help = "receive a file descriptor via SCM rights and assign it a name",
>> - .user_print = monitor_user_noop,
>> - .mhandler.cmd_new = do_getfd,
>> + .mhandler.cmd = hmp_getfd,
>> },
>>
>> STEXI
>> @@ -1253,8 +1252,7 @@ ETEXI
>> .args_type = "fdname:s",
>> .params = "closefd name",
>> .help = "close a file descriptor previously passed via SCM rights",
>> - .user_print = monitor_user_noop,
>> - .mhandler.cmd_new = do_closefd,
>> + .mhandler.cmd = hmp_closefd,
>> },
>>
>> STEXI
>> diff --git a/hmp.c b/hmp.c
>> index 2ce8cb9..6241856 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -999,3 +999,21 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
>> qmp_netdev_del(id, &err);
>> hmp_handle_error(mon, &err);
>> }
>> +
>> +void hmp_getfd(Monitor *mon, const QDict *qdict)
>> +{
>> + const char *fdname = qdict_get_str(qdict, "fdname");
>> + Error *errp = NULL;
>> +
>> + qmp_getfd(fdname, &errp);
>> + hmp_handle_error(mon, &errp);
>> +}
>> +
>> +void hmp_closefd(Monitor *mon, const QDict *qdict)
>> +{
>> + const char *fdname = qdict_get_str(qdict, "fdname");
>> + Error *errp = NULL;
>> +
>> + qmp_closefd(fdname, &errp);
>> + hmp_handle_error(mon, &errp);
>> +}
>> diff --git a/hmp.h b/hmp.h
>> index 79d138d..8d2b0d7 100644
>> --- a/hmp.h
>> +++ b/hmp.h
>> @@ -64,5 +64,7 @@ void hmp_device_del(Monitor *mon, const QDict *qdict);
>> void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
>> void hmp_netdev_add(Monitor *mon, const QDict *qdict);
>> void hmp_netdev_del(Monitor *mon, const QDict *qdict);
>> +void hmp_getfd(Monitor *mon, const QDict *qdict);
>> +void hmp_closefd(Monitor *mon, const QDict *qdict);
>>
>> #endif
>> diff --git a/monitor.c b/monitor.c
>> index a3bc2c7..4c53cb6 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -2182,48 +2182,46 @@ static void do_inject_mce(Monitor *mon, const QDict *qdict)
>> }
>> #endif
>>
>> -static int do_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
>> +void qmp_getfd(const char *fdname, Error **errp)
>> {
>> - const char *fdname = qdict_get_str(qdict, "fdname");
>> mon_fd_t *monfd;
>> int fd;
>>
>> - fd = qemu_chr_fe_get_msgfd(mon->chr);
>> + fd = qemu_chr_fe_get_msgfd(cur_mon->chr);
>> if (fd == -1) {
>> - qerror_report(QERR_FD_NOT_SUPPLIED);
>> - return -1;
>> + error_set(errp, QERR_FD_NOT_SUPPLIED);
>> + return;
>> }
>>
>> if (qemu_isdigit(fdname[0])) {
>> - qerror_report(QERR_INVALID_PARAMETER_VALUE, "fdname",
>> - "a name not starting with a digit");
>> - return -1;
>> + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdname",
>> + "a name not starting with a digit");
>> + return;
>> }
>>
>> - QLIST_FOREACH(monfd, &mon->fds, next) {
>> + QLIST_FOREACH(monfd, &cur_mon->fds, next) {
>> if (strcmp(monfd->name, fdname) != 0) {
>> continue;
>> }
>>
>> close(monfd->fd);
>> monfd->fd = fd;
>> - return 0;
>> + return;
>> }
>>
>> monfd = g_malloc0(sizeof(mon_fd_t));
>> monfd->name = g_strdup(fdname);
>> monfd->fd = fd;
>>
>> - QLIST_INSERT_HEAD(&mon->fds, monfd, next);
>> - return 0;
>> + QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
>> + return;
>
> You have a few of this kind of gratuitous return, please drop 'em.
>
Thanks for reviewing.
I'll get a v3 patch series going and remove the unnecessary returns in it.
>> }
>>
>> -static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
>> +void qmp_closefd(const char *fdname, Error **errp)
>> {
>> - const char *fdname = qdict_get_str(qdict, "fdname");
>> mon_fd_t *monfd;
>>
>> - QLIST_FOREACH(monfd, &mon->fds, next) {
>> + QLIST_FOREACH(monfd, &cur_mon->fds, next) {
>> if (strcmp(monfd->name, fdname) != 0) {
>> continue;
>> }
>> @@ -2232,11 +2230,11 @@ static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
>> close(monfd->fd);
>> g_free(monfd->name);
>> g_free(monfd);
>> - return 0;
>> + return;
>> }
>>
>> - qerror_report(QERR_FD_NOT_FOUND, fdname);
>> - return -1;
>> + error_set(errp, QERR_FD_NOT_FOUND, fdname);
>> + return;
>> }
>>
>> static void do_loadvm(Monitor *mon, const QDict *qdict)
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 3b6e346..6be1d90 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -1862,3 +1862,32 @@
>> # Since: 0.14.0
>> ##
>> { 'command': 'netdev_del', 'data': {'id': 'str'} }
>> +
>> +##
>> +# @getfd:
>> +#
>> +# Receive a file descriptor via SCM rights and assign it a name
>> +#
>> +# @fdname: file descriptor name
>> +#
>> +# Returns: Nothing on success
>> +# If file descriptor was not received, FdNotSupplied
>> +# If @fdname is not valid, InvalidParameterType
>
> Please, add a note explaining that getfd automatically closes fds if
> fdname matches an existing name.
Ok
>
> Otherwise this conversion looks very good to me.
Good to hear!
>
>> +#
>> +# Since: 0.14.0
>> +##
>> +{ 'command': 'getfd', 'data': {'fdname': 'str'} }
>> +
>> +##
>> +# @closefd:
>> +#
>> +# Close a file descriptor previously passed via SCM rights
>> +#
>> +# @fdname: file descriptor name
>> +#
>> +# Returns: Nothing on success
>> +# If @fdname is not found, FdNotFound
>> +#
>> +# Since: 0.14.0
>> +##
>> +{ 'command': 'closefd', 'data': {'fdname': 'str'} }
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index 2e1a38e..f8c0f68 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -873,8 +873,7 @@ EQMP
>> .args_type = "fdname:s",
>> .params = "getfd name",
>> .help = "receive a file descriptor via SCM rights and assign it a name",
>> - .user_print = monitor_user_noop,
>> - .mhandler.cmd_new = do_getfd,
>> + .mhandler.cmd_new = qmp_marshal_input_getfd,
>> },
>>
>> SQMP
>> @@ -899,8 +898,7 @@ EQMP
>> .args_type = "fdname:s",
>> .params = "closefd name",
>> .help = "close a file descriptor previously passed via SCM rights",
>> - .user_print = monitor_user_noop,
>> - .mhandler.cmd_new = do_closefd,
>> + .mhandler.cmd_new = qmp_marshal_input_closefd,
>> },
>>
>> SQMP
>
--
Regards,
Corey
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/4] qapi: Convert getfd and closefd
2012-06-08 15:42 ` [Qemu-devel] [PATCH v2 1/4] qapi: Convert getfd and closefd Corey Bryant
2012-06-13 19:41 ` Luiz Capitulino
@ 2012-06-13 19:42 ` Luiz Capitulino
2012-06-13 20:17 ` Corey Bryant
1 sibling, 1 reply; 24+ messages in thread
From: Luiz Capitulino @ 2012-06-13 19:42 UTC (permalink / raw)
To: Corey Bryant; +Cc: kwolf, aliguori, stefanha, libvir-list, qemu-devel, eblake
On Fri, 8 Jun 2012 11:42:56 -0400
Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
> v2:
> - Convert getfd and closefd to QAPI (lcapitulino@redhat.com)
> - Remove changes that returned fd from getfd (lcapitulino@redhat.com)
> - Wrap hmp_* functions around qmp_* functions (kwolf@redhat.com)
> - Move hmp_* functions to hmp.c (lcapitulino@redhat.com)
> - Drop .user_print lines (lcapitulino@redhat.com)
> - Use 'cmd' instead of 'cmd_new' for HMP (lcapitulino@redhat.com)
> - Change QMP command existance back to 0.14 (lcapitulino@redhat.com)
Btw, having the changelog like this is not nice because it becomes part
of the history. It's better to move it after the '---' line, so that
git ignores it.
>
> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
> ---
> hmp-commands.hx | 6 ++----
> hmp.c | 18 ++++++++++++++++++
> hmp.h | 2 ++
> monitor.c | 34 ++++++++++++++++------------------
> qapi-schema.json | 29 +++++++++++++++++++++++++++++
> qmp-commands.hx | 6 ++----
> 6 files changed, 69 insertions(+), 26 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index f5d9d91..eea8b32 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1236,8 +1236,7 @@ ETEXI
> .args_type = "fdname:s",
> .params = "getfd name",
> .help = "receive a file descriptor via SCM rights and assign it a name",
> - .user_print = monitor_user_noop,
> - .mhandler.cmd_new = do_getfd,
> + .mhandler.cmd = hmp_getfd,
> },
>
> STEXI
> @@ -1253,8 +1252,7 @@ ETEXI
> .args_type = "fdname:s",
> .params = "closefd name",
> .help = "close a file descriptor previously passed via SCM rights",
> - .user_print = monitor_user_noop,
> - .mhandler.cmd_new = do_closefd,
> + .mhandler.cmd = hmp_closefd,
> },
>
> STEXI
> diff --git a/hmp.c b/hmp.c
> index 2ce8cb9..6241856 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -999,3 +999,21 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
> qmp_netdev_del(id, &err);
> hmp_handle_error(mon, &err);
> }
> +
> +void hmp_getfd(Monitor *mon, const QDict *qdict)
> +{
> + const char *fdname = qdict_get_str(qdict, "fdname");
> + Error *errp = NULL;
> +
> + qmp_getfd(fdname, &errp);
> + hmp_handle_error(mon, &errp);
> +}
> +
> +void hmp_closefd(Monitor *mon, const QDict *qdict)
> +{
> + const char *fdname = qdict_get_str(qdict, "fdname");
> + Error *errp = NULL;
> +
> + qmp_closefd(fdname, &errp);
> + hmp_handle_error(mon, &errp);
> +}
> diff --git a/hmp.h b/hmp.h
> index 79d138d..8d2b0d7 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -64,5 +64,7 @@ void hmp_device_del(Monitor *mon, const QDict *qdict);
> void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
> void hmp_netdev_add(Monitor *mon, const QDict *qdict);
> void hmp_netdev_del(Monitor *mon, const QDict *qdict);
> +void hmp_getfd(Monitor *mon, const QDict *qdict);
> +void hmp_closefd(Monitor *mon, const QDict *qdict);
>
> #endif
> diff --git a/monitor.c b/monitor.c
> index a3bc2c7..4c53cb6 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2182,48 +2182,46 @@ static void do_inject_mce(Monitor *mon, const QDict *qdict)
> }
> #endif
>
> -static int do_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
> +void qmp_getfd(const char *fdname, Error **errp)
> {
> - const char *fdname = qdict_get_str(qdict, "fdname");
> mon_fd_t *monfd;
> int fd;
>
> - fd = qemu_chr_fe_get_msgfd(mon->chr);
> + fd = qemu_chr_fe_get_msgfd(cur_mon->chr);
> if (fd == -1) {
> - qerror_report(QERR_FD_NOT_SUPPLIED);
> - return -1;
> + error_set(errp, QERR_FD_NOT_SUPPLIED);
> + return;
> }
>
> if (qemu_isdigit(fdname[0])) {
> - qerror_report(QERR_INVALID_PARAMETER_VALUE, "fdname",
> - "a name not starting with a digit");
> - return -1;
> + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdname",
> + "a name not starting with a digit");
> + return;
> }
>
> - QLIST_FOREACH(monfd, &mon->fds, next) {
> + QLIST_FOREACH(monfd, &cur_mon->fds, next) {
> if (strcmp(monfd->name, fdname) != 0) {
> continue;
> }
>
> close(monfd->fd);
> monfd->fd = fd;
> - return 0;
> + return;
> }
>
> monfd = g_malloc0(sizeof(mon_fd_t));
> monfd->name = g_strdup(fdname);
> monfd->fd = fd;
>
> - QLIST_INSERT_HEAD(&mon->fds, monfd, next);
> - return 0;
> + QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
> + return;
> }
>
> -static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
> +void qmp_closefd(const char *fdname, Error **errp)
> {
> - const char *fdname = qdict_get_str(qdict, "fdname");
> mon_fd_t *monfd;
>
> - QLIST_FOREACH(monfd, &mon->fds, next) {
> + QLIST_FOREACH(monfd, &cur_mon->fds, next) {
> if (strcmp(monfd->name, fdname) != 0) {
> continue;
> }
> @@ -2232,11 +2230,11 @@ static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
> close(monfd->fd);
> g_free(monfd->name);
> g_free(monfd);
> - return 0;
> + return;
> }
>
> - qerror_report(QERR_FD_NOT_FOUND, fdname);
> - return -1;
> + error_set(errp, QERR_FD_NOT_FOUND, fdname);
> + return;
> }
>
> static void do_loadvm(Monitor *mon, const QDict *qdict)
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 3b6e346..6be1d90 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1862,3 +1862,32 @@
> # Since: 0.14.0
> ##
> { 'command': 'netdev_del', 'data': {'id': 'str'} }
> +
> +##
> +# @getfd:
> +#
> +# Receive a file descriptor via SCM rights and assign it a name
> +#
> +# @fdname: file descriptor name
> +#
> +# Returns: Nothing on success
> +# If file descriptor was not received, FdNotSupplied
> +# If @fdname is not valid, InvalidParameterType
> +#
> +# Since: 0.14.0
> +##
> +{ 'command': 'getfd', 'data': {'fdname': 'str'} }
> +
> +##
> +# @closefd:
> +#
> +# Close a file descriptor previously passed via SCM rights
> +#
> +# @fdname: file descriptor name
> +#
> +# Returns: Nothing on success
> +# If @fdname is not found, FdNotFound
> +#
> +# Since: 0.14.0
> +##
> +{ 'command': 'closefd', 'data': {'fdname': 'str'} }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 2e1a38e..f8c0f68 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -873,8 +873,7 @@ EQMP
> .args_type = "fdname:s",
> .params = "getfd name",
> .help = "receive a file descriptor via SCM rights and assign it a name",
> - .user_print = monitor_user_noop,
> - .mhandler.cmd_new = do_getfd,
> + .mhandler.cmd_new = qmp_marshal_input_getfd,
> },
>
> SQMP
> @@ -899,8 +898,7 @@ EQMP
> .args_type = "fdname:s",
> .params = "closefd name",
> .help = "close a file descriptor previously passed via SCM rights",
> - .user_print = monitor_user_noop,
> - .mhandler.cmd_new = do_closefd,
> + .mhandler.cmd_new = qmp_marshal_input_closefd,
> },
>
> SQMP
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH v2 1/4] qapi: Convert getfd and closefd
2012-06-13 19:42 ` Luiz Capitulino
@ 2012-06-13 20:17 ` Corey Bryant
2012-06-13 20:41 ` Luiz Capitulino
2012-06-13 20:41 ` Eric Blake
0 siblings, 2 replies; 24+ messages in thread
From: Corey Bryant @ 2012-06-13 20:17 UTC (permalink / raw)
To: Luiz Capitulino
Cc: kwolf, aliguori, stefanha, libvir-list, qemu-devel, eblake
On 06/13/2012 03:42 PM, Luiz Capitulino wrote:
> On Fri, 8 Jun 2012 11:42:56 -0400
> Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
>
>> v2:
>> - Convert getfd and closefd to QAPI (lcapitulino@redhat.com)
>> - Remove changes that returned fd from getfd (lcapitulino@redhat.com)
>> - Wrap hmp_* functions around qmp_* functions (kwolf@redhat.com)
>> - Move hmp_* functions to hmp.c (lcapitulino@redhat.com)
>> - Drop .user_print lines (lcapitulino@redhat.com)
>> - Use 'cmd' instead of 'cmd_new' for HMP (lcapitulino@redhat.com)
>> - Change QMP command existance back to 0.14 (lcapitulino@redhat.com)
>
> Btw, having the changelog like this is not nice because it becomes part
> of the history. It's better to move it after the '---' line, so that
> git ignores it.
>
I see your point and I can do this in v3. But can I add text after the
'---' line in the commit message via 'git commit' or do I have to
manually edit the patches?
>>
>> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
>> ---
>> hmp-commands.hx | 6 ++----
>> hmp.c | 18 ++++++++++++++++++
>> hmp.h | 2 ++
>> monitor.c | 34 ++++++++++++++++------------------
>> qapi-schema.json | 29 +++++++++++++++++++++++++++++
>> qmp-commands.hx | 6 ++----
>> 6 files changed, 69 insertions(+), 26 deletions(-)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index f5d9d91..eea8b32 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -1236,8 +1236,7 @@ ETEXI
>> .args_type = "fdname:s",
>> .params = "getfd name",
>> .help = "receive a file descriptor via SCM rights and assign it a name",
>> - .user_print = monitor_user_noop,
>> - .mhandler.cmd_new = do_getfd,
>> + .mhandler.cmd = hmp_getfd,
>> },
>>
>> STEXI
>> @@ -1253,8 +1252,7 @@ ETEXI
>> .args_type = "fdname:s",
>> .params = "closefd name",
>> .help = "close a file descriptor previously passed via SCM rights",
>> - .user_print = monitor_user_noop,
>> - .mhandler.cmd_new = do_closefd,
>> + .mhandler.cmd = hmp_closefd,
>> },
>>
>> STEXI
>> diff --git a/hmp.c b/hmp.c
>> index 2ce8cb9..6241856 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -999,3 +999,21 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
>> qmp_netdev_del(id, &err);
>> hmp_handle_error(mon, &err);
>> }
>> +
>> +void hmp_getfd(Monitor *mon, const QDict *qdict)
>> +{
>> + const char *fdname = qdict_get_str(qdict, "fdname");
>> + Error *errp = NULL;
>> +
>> + qmp_getfd(fdname, &errp);
>> + hmp_handle_error(mon, &errp);
>> +}
>> +
>> +void hmp_closefd(Monitor *mon, const QDict *qdict)
>> +{
>> + const char *fdname = qdict_get_str(qdict, "fdname");
>> + Error *errp = NULL;
>> +
>> + qmp_closefd(fdname, &errp);
>> + hmp_handle_error(mon, &errp);
>> +}
>> diff --git a/hmp.h b/hmp.h
>> index 79d138d..8d2b0d7 100644
>> --- a/hmp.h
>> +++ b/hmp.h
>> @@ -64,5 +64,7 @@ void hmp_device_del(Monitor *mon, const QDict *qdict);
>> void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
>> void hmp_netdev_add(Monitor *mon, const QDict *qdict);
>> void hmp_netdev_del(Monitor *mon, const QDict *qdict);
>> +void hmp_getfd(Monitor *mon, const QDict *qdict);
>> +void hmp_closefd(Monitor *mon, const QDict *qdict);
>>
>> #endif
>> diff --git a/monitor.c b/monitor.c
>> index a3bc2c7..4c53cb6 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -2182,48 +2182,46 @@ static void do_inject_mce(Monitor *mon, const QDict *qdict)
>> }
>> #endif
>>
>> -static int do_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
>> +void qmp_getfd(const char *fdname, Error **errp)
>> {
>> - const char *fdname = qdict_get_str(qdict, "fdname");
>> mon_fd_t *monfd;
>> int fd;
>>
>> - fd = qemu_chr_fe_get_msgfd(mon->chr);
>> + fd = qemu_chr_fe_get_msgfd(cur_mon->chr);
>> if (fd == -1) {
>> - qerror_report(QERR_FD_NOT_SUPPLIED);
>> - return -1;
>> + error_set(errp, QERR_FD_NOT_SUPPLIED);
>> + return;
>> }
>>
>> if (qemu_isdigit(fdname[0])) {
>> - qerror_report(QERR_INVALID_PARAMETER_VALUE, "fdname",
>> - "a name not starting with a digit");
>> - return -1;
>> + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdname",
>> + "a name not starting with a digit");
>> + return;
>> }
>>
>> - QLIST_FOREACH(monfd, &mon->fds, next) {
>> + QLIST_FOREACH(monfd, &cur_mon->fds, next) {
>> if (strcmp(monfd->name, fdname) != 0) {
>> continue;
>> }
>>
>> close(monfd->fd);
>> monfd->fd = fd;
>> - return 0;
>> + return;
>> }
>>
>> monfd = g_malloc0(sizeof(mon_fd_t));
>> monfd->name = g_strdup(fdname);
>> monfd->fd = fd;
>>
>> - QLIST_INSERT_HEAD(&mon->fds, monfd, next);
>> - return 0;
>> + QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
>> + return;
>> }
>>
>> -static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
>> +void qmp_closefd(const char *fdname, Error **errp)
>> {
>> - const char *fdname = qdict_get_str(qdict, "fdname");
>> mon_fd_t *monfd;
>>
>> - QLIST_FOREACH(monfd, &mon->fds, next) {
>> + QLIST_FOREACH(monfd, &cur_mon->fds, next) {
>> if (strcmp(monfd->name, fdname) != 0) {
>> continue;
>> }
>> @@ -2232,11 +2230,11 @@ static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
>> close(monfd->fd);
>> g_free(monfd->name);
>> g_free(monfd);
>> - return 0;
>> + return;
>> }
>>
>> - qerror_report(QERR_FD_NOT_FOUND, fdname);
>> - return -1;
>> + error_set(errp, QERR_FD_NOT_FOUND, fdname);
>> + return;
>> }
>>
>> static void do_loadvm(Monitor *mon, const QDict *qdict)
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 3b6e346..6be1d90 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -1862,3 +1862,32 @@
>> # Since: 0.14.0
>> ##
>> { 'command': 'netdev_del', 'data': {'id': 'str'} }
>> +
>> +##
>> +# @getfd:
>> +#
>> +# Receive a file descriptor via SCM rights and assign it a name
>> +#
>> +# @fdname: file descriptor name
>> +#
>> +# Returns: Nothing on success
>> +# If file descriptor was not received, FdNotSupplied
>> +# If @fdname is not valid, InvalidParameterType
>> +#
>> +# Since: 0.14.0
>> +##
>> +{ 'command': 'getfd', 'data': {'fdname': 'str'} }
>> +
>> +##
>> +# @closefd:
>> +#
>> +# Close a file descriptor previously passed via SCM rights
>> +#
>> +# @fdname: file descriptor name
>> +#
>> +# Returns: Nothing on success
>> +# If @fdname is not found, FdNotFound
>> +#
>> +# Since: 0.14.0
>> +##
>> +{ 'command': 'closefd', 'data': {'fdname': 'str'} }
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index 2e1a38e..f8c0f68 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -873,8 +873,7 @@ EQMP
>> .args_type = "fdname:s",
>> .params = "getfd name",
>> .help = "receive a file descriptor via SCM rights and assign it a name",
>> - .user_print = monitor_user_noop,
>> - .mhandler.cmd_new = do_getfd,
>> + .mhandler.cmd_new = qmp_marshal_input_getfd,
>> },
>>
>> SQMP
>> @@ -899,8 +898,7 @@ EQMP
>> .args_type = "fdname:s",
>> .params = "closefd name",
>> .help = "close a file descriptor previously passed via SCM rights",
>> - .user_print = monitor_user_noop,
>> - .mhandler.cmd_new = do_closefd,
>> + .mhandler.cmd_new = qmp_marshal_input_closefd,
>> },
>>
>> SQMP
>
>
--
Regards,
Corey
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH v2 1/4] qapi: Convert getfd and closefd
2012-06-13 20:17 ` Corey Bryant
@ 2012-06-13 20:41 ` Luiz Capitulino
2012-06-13 20:41 ` Eric Blake
1 sibling, 0 replies; 24+ messages in thread
From: Luiz Capitulino @ 2012-06-13 20:41 UTC (permalink / raw)
To: Corey Bryant; +Cc: kwolf, aliguori, stefanha, libvir-list, qemu-devel, eblake
On Wed, 13 Jun 2012 16:17:30 -0400
Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
>
>
> On 06/13/2012 03:42 PM, Luiz Capitulino wrote:
> > On Fri, 8 Jun 2012 11:42:56 -0400
> > Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
> >
> >> v2:
> >> - Convert getfd and closefd to QAPI (lcapitulino@redhat.com)
> >> - Remove changes that returned fd from getfd (lcapitulino@redhat.com)
> >> - Wrap hmp_* functions around qmp_* functions (kwolf@redhat.com)
> >> - Move hmp_* functions to hmp.c (lcapitulino@redhat.com)
> >> - Drop .user_print lines (lcapitulino@redhat.com)
> >> - Use 'cmd' instead of 'cmd_new' for HMP (lcapitulino@redhat.com)
> >> - Change QMP command existance back to 0.14 (lcapitulino@redhat.com)
> >
> > Btw, having the changelog like this is not nice because it becomes part
> > of the history. It's better to move it after the '---' line, so that
> > git ignores it.
> >
>
> I see your point and I can do this in v3. But can I add text after the
> '---' line in the commit message via 'git commit' or do I have to
> manually edit the patches?
I don't know of an automated way of doing this, I'd export the patches with
git format-patch and edit them manually.
But I usually put changelog in the intro patch.
>
> >>
> >> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
> >> ---
> >> hmp-commands.hx | 6 ++----
> >> hmp.c | 18 ++++++++++++++++++
> >> hmp.h | 2 ++
> >> monitor.c | 34 ++++++++++++++++------------------
> >> qapi-schema.json | 29 +++++++++++++++++++++++++++++
> >> qmp-commands.hx | 6 ++----
> >> 6 files changed, 69 insertions(+), 26 deletions(-)
> >>
> >> diff --git a/hmp-commands.hx b/hmp-commands.hx
> >> index f5d9d91..eea8b32 100644
> >> --- a/hmp-commands.hx
> >> +++ b/hmp-commands.hx
> >> @@ -1236,8 +1236,7 @@ ETEXI
> >> .args_type = "fdname:s",
> >> .params = "getfd name",
> >> .help = "receive a file descriptor via SCM rights and assign it a name",
> >> - .user_print = monitor_user_noop,
> >> - .mhandler.cmd_new = do_getfd,
> >> + .mhandler.cmd = hmp_getfd,
> >> },
> >>
> >> STEXI
> >> @@ -1253,8 +1252,7 @@ ETEXI
> >> .args_type = "fdname:s",
> >> .params = "closefd name",
> >> .help = "close a file descriptor previously passed via SCM rights",
> >> - .user_print = monitor_user_noop,
> >> - .mhandler.cmd_new = do_closefd,
> >> + .mhandler.cmd = hmp_closefd,
> >> },
> >>
> >> STEXI
> >> diff --git a/hmp.c b/hmp.c
> >> index 2ce8cb9..6241856 100644
> >> --- a/hmp.c
> >> +++ b/hmp.c
> >> @@ -999,3 +999,21 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
> >> qmp_netdev_del(id, &err);
> >> hmp_handle_error(mon, &err);
> >> }
> >> +
> >> +void hmp_getfd(Monitor *mon, const QDict *qdict)
> >> +{
> >> + const char *fdname = qdict_get_str(qdict, "fdname");
> >> + Error *errp = NULL;
> >> +
> >> + qmp_getfd(fdname, &errp);
> >> + hmp_handle_error(mon, &errp);
> >> +}
> >> +
> >> +void hmp_closefd(Monitor *mon, const QDict *qdict)
> >> +{
> >> + const char *fdname = qdict_get_str(qdict, "fdname");
> >> + Error *errp = NULL;
> >> +
> >> + qmp_closefd(fdname, &errp);
> >> + hmp_handle_error(mon, &errp);
> >> +}
> >> diff --git a/hmp.h b/hmp.h
> >> index 79d138d..8d2b0d7 100644
> >> --- a/hmp.h
> >> +++ b/hmp.h
> >> @@ -64,5 +64,7 @@ void hmp_device_del(Monitor *mon, const QDict *qdict);
> >> void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
> >> void hmp_netdev_add(Monitor *mon, const QDict *qdict);
> >> void hmp_netdev_del(Monitor *mon, const QDict *qdict);
> >> +void hmp_getfd(Monitor *mon, const QDict *qdict);
> >> +void hmp_closefd(Monitor *mon, const QDict *qdict);
> >>
> >> #endif
> >> diff --git a/monitor.c b/monitor.c
> >> index a3bc2c7..4c53cb6 100644
> >> --- a/monitor.c
> >> +++ b/monitor.c
> >> @@ -2182,48 +2182,46 @@ static void do_inject_mce(Monitor *mon, const QDict *qdict)
> >> }
> >> #endif
> >>
> >> -static int do_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
> >> +void qmp_getfd(const char *fdname, Error **errp)
> >> {
> >> - const char *fdname = qdict_get_str(qdict, "fdname");
> >> mon_fd_t *monfd;
> >> int fd;
> >>
> >> - fd = qemu_chr_fe_get_msgfd(mon->chr);
> >> + fd = qemu_chr_fe_get_msgfd(cur_mon->chr);
> >> if (fd == -1) {
> >> - qerror_report(QERR_FD_NOT_SUPPLIED);
> >> - return -1;
> >> + error_set(errp, QERR_FD_NOT_SUPPLIED);
> >> + return;
> >> }
> >>
> >> if (qemu_isdigit(fdname[0])) {
> >> - qerror_report(QERR_INVALID_PARAMETER_VALUE, "fdname",
> >> - "a name not starting with a digit");
> >> - return -1;
> >> + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdname",
> >> + "a name not starting with a digit");
> >> + return;
> >> }
> >>
> >> - QLIST_FOREACH(monfd, &mon->fds, next) {
> >> + QLIST_FOREACH(monfd, &cur_mon->fds, next) {
> >> if (strcmp(monfd->name, fdname) != 0) {
> >> continue;
> >> }
> >>
> >> close(monfd->fd);
> >> monfd->fd = fd;
> >> - return 0;
> >> + return;
> >> }
> >>
> >> monfd = g_malloc0(sizeof(mon_fd_t));
> >> monfd->name = g_strdup(fdname);
> >> monfd->fd = fd;
> >>
> >> - QLIST_INSERT_HEAD(&mon->fds, monfd, next);
> >> - return 0;
> >> + QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
> >> + return;
> >> }
> >>
> >> -static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
> >> +void qmp_closefd(const char *fdname, Error **errp)
> >> {
> >> - const char *fdname = qdict_get_str(qdict, "fdname");
> >> mon_fd_t *monfd;
> >>
> >> - QLIST_FOREACH(monfd, &mon->fds, next) {
> >> + QLIST_FOREACH(monfd, &cur_mon->fds, next) {
> >> if (strcmp(monfd->name, fdname) != 0) {
> >> continue;
> >> }
> >> @@ -2232,11 +2230,11 @@ static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
> >> close(monfd->fd);
> >> g_free(monfd->name);
> >> g_free(monfd);
> >> - return 0;
> >> + return;
> >> }
> >>
> >> - qerror_report(QERR_FD_NOT_FOUND, fdname);
> >> - return -1;
> >> + error_set(errp, QERR_FD_NOT_FOUND, fdname);
> >> + return;
> >> }
> >>
> >> static void do_loadvm(Monitor *mon, const QDict *qdict)
> >> diff --git a/qapi-schema.json b/qapi-schema.json
> >> index 3b6e346..6be1d90 100644
> >> --- a/qapi-schema.json
> >> +++ b/qapi-schema.json
> >> @@ -1862,3 +1862,32 @@
> >> # Since: 0.14.0
> >> ##
> >> { 'command': 'netdev_del', 'data': {'id': 'str'} }
> >> +
> >> +##
> >> +# @getfd:
> >> +#
> >> +# Receive a file descriptor via SCM rights and assign it a name
> >> +#
> >> +# @fdname: file descriptor name
> >> +#
> >> +# Returns: Nothing on success
> >> +# If file descriptor was not received, FdNotSupplied
> >> +# If @fdname is not valid, InvalidParameterType
> >> +#
> >> +# Since: 0.14.0
> >> +##
> >> +{ 'command': 'getfd', 'data': {'fdname': 'str'} }
> >> +
> >> +##
> >> +# @closefd:
> >> +#
> >> +# Close a file descriptor previously passed via SCM rights
> >> +#
> >> +# @fdname: file descriptor name
> >> +#
> >> +# Returns: Nothing on success
> >> +# If @fdname is not found, FdNotFound
> >> +#
> >> +# Since: 0.14.0
> >> +##
> >> +{ 'command': 'closefd', 'data': {'fdname': 'str'} }
> >> diff --git a/qmp-commands.hx b/qmp-commands.hx
> >> index 2e1a38e..f8c0f68 100644
> >> --- a/qmp-commands.hx
> >> +++ b/qmp-commands.hx
> >> @@ -873,8 +873,7 @@ EQMP
> >> .args_type = "fdname:s",
> >> .params = "getfd name",
> >> .help = "receive a file descriptor via SCM rights and assign it a name",
> >> - .user_print = monitor_user_noop,
> >> - .mhandler.cmd_new = do_getfd,
> >> + .mhandler.cmd_new = qmp_marshal_input_getfd,
> >> },
> >>
> >> SQMP
> >> @@ -899,8 +898,7 @@ EQMP
> >> .args_type = "fdname:s",
> >> .params = "closefd name",
> >> .help = "close a file descriptor previously passed via SCM rights",
> >> - .user_print = monitor_user_noop,
> >> - .mhandler.cmd_new = do_closefd,
> >> + .mhandler.cmd_new = qmp_marshal_input_closefd,
> >> },
> >>
> >> SQMP
> >
> >
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH v2 1/4] qapi: Convert getfd and closefd
2012-06-13 20:17 ` Corey Bryant
2012-06-13 20:41 ` Luiz Capitulino
@ 2012-06-13 20:41 ` Eric Blake
2012-06-13 21:43 ` Corey Bryant
1 sibling, 1 reply; 24+ messages in thread
From: Eric Blake @ 2012-06-13 20:41 UTC (permalink / raw)
To: Corey Bryant
Cc: kwolf, aliguori, stefanha, libvir-list, qemu-devel,
Luiz Capitulino
[-- Attachment #1: Type: text/plain, Size: 2264 bytes --]
On 06/13/2012 02:17 PM, Corey Bryant wrote:
>
>
> On 06/13/2012 03:42 PM, Luiz Capitulino wrote:
>> On Fri, 8 Jun 2012 11:42:56 -0400
>> Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
>>
>>> v2:
>>> - Convert getfd and closefd to QAPI (lcapitulino@redhat.com)
>>> - Remove changes that returned fd from getfd (lcapitulino@redhat.com)
>>> - Wrap hmp_* functions around qmp_* functions (kwolf@redhat.com)
>>> - Move hmp_* functions to hmp.c (lcapitulino@redhat.com)
>>> - Drop .user_print lines (lcapitulino@redhat.com)
>>> - Use 'cmd' instead of 'cmd_new' for HMP (lcapitulino@redhat.com)
>>> - Change QMP command existance back to 0.14 (lcapitulino@redhat.com)
>>
>> Btw, having the changelog like this is not nice because it becomes part
>> of the history. It's better to move it after the '---' line, so that
>> git ignores it.
>>
>
> I see your point and I can do this in v3. But can I add text after the
> '---' line in the commit message via 'git commit' or do I have to
> manually edit the patches?
I also like tracking my notes to self/reviewers in the commit message as
I do a rebase. 'git send-email' automatically adds --- at the end of
your commit message, so I personally end up using 'git send-email
--annotate' and manually move the --- line to occur before my separation
point. I think you can also stick --- in the middle of your commit
message at which point 'git am' will truncate from the first instance
when applying your email, without you having to edit things when
mailing, although I haven't tried it myself (at any rate, I have seen
patches from others with double --- lines, and assume that the doubled
line is a result of the literal --- line in their commit message).
Supposedly, it is also possible to use 'git notes' coupled with
notes.rewrite* options in your .gitconfig to track your notes over a
rebase, as well as an undocumented option to 'git send-email' to have
your notes automatically included after a lone --- line, but that are of
git is woefully under-documented and probably has issues that need
fixing before turning it into a daily workflow.
--
Eric Blake eblake@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/4] qapi: Convert getfd and closefd
2012-06-13 20:41 ` Eric Blake
@ 2012-06-13 21:43 ` Corey Bryant
0 siblings, 0 replies; 24+ messages in thread
From: Corey Bryant @ 2012-06-13 21:43 UTC (permalink / raw)
To: Eric Blake
Cc: kwolf, aliguori, stefanha, libvir-list, qemu-devel,
Luiz Capitulino
On 06/13/2012 04:41 PM, Eric Blake wrote:
> On 06/13/2012 02:17 PM, Corey Bryant wrote:
>>
>>
>> On 06/13/2012 03:42 PM, Luiz Capitulino wrote:
>>> On Fri, 8 Jun 2012 11:42:56 -0400
>>> Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
>>>
>>>> v2:
>>>> - Convert getfd and closefd to QAPI (lcapitulino@redhat.com)
>>>> - Remove changes that returned fd from getfd (lcapitulino@redhat.com)
>>>> - Wrap hmp_* functions around qmp_* functions (kwolf@redhat.com)
>>>> - Move hmp_* functions to hmp.c (lcapitulino@redhat.com)
>>>> - Drop .user_print lines (lcapitulino@redhat.com)
>>>> - Use 'cmd' instead of 'cmd_new' for HMP (lcapitulino@redhat.com)
>>>> - Change QMP command existance back to 0.14 (lcapitulino@redhat.com)
>>>
>>> Btw, having the changelog like this is not nice because it becomes part
>>> of the history. It's better to move it after the '---' line, so that
>>> git ignores it.
>>>
>>
>> I see your point and I can do this in v3. But can I add text after the
>> '---' line in the commit message via 'git commit' or do I have to
>> manually edit the patches?
>
> I also like tracking my notes to self/reviewers in the commit message as
> I do a rebase. 'git send-email' automatically adds --- at the end of
> your commit message, so I personally end up using 'git send-email
> --annotate' and manually move the --- line to occur before my separation
> point. I think you can also stick --- in the middle of your commit
> message at which point 'git am' will truncate from the first instance
> when applying your email, without you having to edit things when
> mailing, although I haven't tried it myself (at any rate, I have seen
> patches from others with double --- lines, and assume that the doubled
> line is a result of the literal --- line in their commit message).
>
> Supposedly, it is also possible to use 'git notes' coupled with
> notes.rewrite* options in your .gitconfig to track your notes over a
> rebase, as well as an undocumented option to 'git send-email' to have
> your notes automatically included after a lone --- line, but that are of
> git is woefully under-documented and probably has issues that need
> fixing before turning it into a daily workflow.
>
Thanks for the tips!
--
Regards,
Corey
^ permalink raw reply [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v2 2/4] qapi: Add passfd QMP command
2012-06-08 15:42 [Qemu-devel] [PATCH v2 0/4] file descriptor passing using passfd Corey Bryant
2012-06-08 15:42 ` [Qemu-devel] [PATCH v2 1/4] qapi: Convert getfd and closefd Corey Bryant
@ 2012-06-08 15:42 ` Corey Bryant
2012-06-13 19:46 ` Luiz Capitulino
2012-06-08 15:42 ` [Qemu-devel] [PATCH v2 3/4] osdep: Enable qemu_open to dup pre-opened fd Corey Bryant
` (3 subsequent siblings)
5 siblings, 1 reply; 24+ messages in thread
From: Corey Bryant @ 2012-06-08 15:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, aliguori, stefanha, libvir-list, Corey Bryant, eblake
This patch adds the passfd QMP command using the QAPI framework.
Like the getfd command, it is used to pass a file descriptor via
SCM_RIGHTS. However, the passfd command also returns the received
file descriptor, which is a difference in behavior from the getfd
command, which returns nothing.
The closefd command can be used to close a file descriptor that was
passed with the passfd command.
v2:
-Introduce new QMP command to pass/return fd (lcapitulino@redhat.com)
-Use passfd as command name (berrange@redhat.com)
Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
---
monitor.c | 14 ++++++++++----
qapi-schema.json | 15 +++++++++++++++
qmp-commands.hx | 27 +++++++++++++++++++++++++++
3 files changed, 52 insertions(+), 4 deletions(-)
diff --git a/monitor.c b/monitor.c
index 4c53cb6..980a226 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2182,7 +2182,7 @@ static void do_inject_mce(Monitor *mon, const QDict *qdict)
}
#endif
-void qmp_getfd(const char *fdname, Error **errp)
+int64_t qmp_passfd(const char *fdname, Error **errp)
{
mon_fd_t *monfd;
int fd;
@@ -2190,13 +2190,13 @@ void qmp_getfd(const char *fdname, Error **errp)
fd = qemu_chr_fe_get_msgfd(cur_mon->chr);
if (fd == -1) {
error_set(errp, QERR_FD_NOT_SUPPLIED);
- return;
+ return -1;
}
if (qemu_isdigit(fdname[0])) {
error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdname",
"a name not starting with a digit");
- return;
+ return -1;
}
QLIST_FOREACH(monfd, &cur_mon->fds, next) {
@@ -2206,7 +2206,7 @@ void qmp_getfd(const char *fdname, Error **errp)
close(monfd->fd);
monfd->fd = fd;
- return;
+ return fd;
}
monfd = g_malloc0(sizeof(mon_fd_t));
@@ -2214,6 +2214,12 @@ void qmp_getfd(const char *fdname, Error **errp)
monfd->fd = fd;
QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
+ return fd;
+}
+
+void qmp_getfd(const char *fdname, Error **errp)
+{
+ qmp_passfd(fdname, errp);
return;
}
diff --git a/qapi-schema.json b/qapi-schema.json
index 6be1d90..15da1b8 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1864,6 +1864,21 @@
{ 'command': 'netdev_del', 'data': {'id': 'str'} }
##
+# @passfd:
+#
+# Pass a file descriptor via SCM rights and assign it a name
+#
+# @fdname: file descriptor name
+#
+# Returns: The QEMU file descriptor that was received
+# If file descriptor was not received, FdNotSupplied
+# If @fdname is not valid, InvalidParameterType
+#
+# Since: 1.2.0
+##
+{ 'command': 'passfd', 'data': {'fdname': 'str'}, 'returns': 'int' }
+
+##
# @getfd:
#
# Receive a file descriptor via SCM rights and assign it a name
diff --git a/qmp-commands.hx b/qmp-commands.hx
index f8c0f68..338a0b3 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -869,6 +869,33 @@ Example:
EQMP
{
+ .name = "passfd",
+ .args_type = "fdname:s",
+ .params = "passfd name",
+ .help = "pass a file descriptor via SCM rights and assign it a name",
+ .mhandler.cmd_new = qmp_marshal_input_passfd,
+ },
+
+SQMP
+passfd
+------
+
+Pass a file descriptor via SCM rights and assign it a name.
+
+Arguments:
+
+- "fdname": file descriptor name (json-string)
+
+Return a json-int with the QEMU file descriptor that was received.
+
+Example:
+
+-> { "execute": "passfd", "arguments": { "fdname": "fd1" } }
+<- { "return": 42 }
+
+EQMP
+
+ {
.name = "getfd",
.args_type = "fdname:s",
.params = "getfd name",
--
1.7.10.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH v2 2/4] qapi: Add passfd QMP command
2012-06-08 15:42 ` [Qemu-devel] [PATCH v2 2/4] qapi: Add passfd QMP command Corey Bryant
@ 2012-06-13 19:46 ` Luiz Capitulino
2012-06-13 20:25 ` Corey Bryant
0 siblings, 1 reply; 24+ messages in thread
From: Luiz Capitulino @ 2012-06-13 19:46 UTC (permalink / raw)
To: Corey Bryant; +Cc: kwolf, aliguori, stefanha, libvir-list, qemu-devel, eblake
On Fri, 8 Jun 2012 11:42:57 -0400
Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
> This patch adds the passfd QMP command using the QAPI framework.
> Like the getfd command, it is used to pass a file descriptor via
> SCM_RIGHTS. However, the passfd command also returns the received
> file descriptor, which is a difference in behavior from the getfd
> command, which returns nothing.
Let's call it pass-fd instead.
Also, getfd automatically closes a fd if an existing fdname is passed again.
I don't think this is a good behavior, I think pass-fd should fail instead
(note that we can't fix getfd though).
>
> The closefd command can be used to close a file descriptor that was
> passed with the passfd command.
>
> v2:
> -Introduce new QMP command to pass/return fd (lcapitulino@redhat.com)
> -Use passfd as command name (berrange@redhat.com)
>
> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
> ---
> monitor.c | 14 ++++++++++----
> qapi-schema.json | 15 +++++++++++++++
> qmp-commands.hx | 27 +++++++++++++++++++++++++++
> 3 files changed, 52 insertions(+), 4 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 4c53cb6..980a226 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2182,7 +2182,7 @@ static void do_inject_mce(Monitor *mon, const QDict *qdict)
> }
> #endif
>
> -void qmp_getfd(const char *fdname, Error **errp)
> +int64_t qmp_passfd(const char *fdname, Error **errp)
> {
> mon_fd_t *monfd;
> int fd;
> @@ -2190,13 +2190,13 @@ void qmp_getfd(const char *fdname, Error **errp)
> fd = qemu_chr_fe_get_msgfd(cur_mon->chr);
> if (fd == -1) {
> error_set(errp, QERR_FD_NOT_SUPPLIED);
> - return;
> + return -1;
> }
>
> if (qemu_isdigit(fdname[0])) {
> error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdname",
> "a name not starting with a digit");
> - return;
> + return -1;
> }
>
> QLIST_FOREACH(monfd, &cur_mon->fds, next) {
> @@ -2206,7 +2206,7 @@ void qmp_getfd(const char *fdname, Error **errp)
>
> close(monfd->fd);
> monfd->fd = fd;
> - return;
> + return fd;
> }
>
> monfd = g_malloc0(sizeof(mon_fd_t));
> @@ -2214,6 +2214,12 @@ void qmp_getfd(const char *fdname, Error **errp)
> monfd->fd = fd;
>
> QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
> + return fd;
> +}
> +
> +void qmp_getfd(const char *fdname, Error **errp)
> +{
> + qmp_passfd(fdname, errp);
> return;
> }
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 6be1d90..15da1b8 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1864,6 +1864,21 @@
> { 'command': 'netdev_del', 'data': {'id': 'str'} }
>
> ##
> +# @passfd:
> +#
> +# Pass a file descriptor via SCM rights and assign it a name
> +#
> +# @fdname: file descriptor name
> +#
> +# Returns: The QEMU file descriptor that was received
> +# If file descriptor was not received, FdNotSupplied
> +# If @fdname is not valid, InvalidParameterType
> +#
> +# Since: 1.2.0
> +##
> +{ 'command': 'passfd', 'data': {'fdname': 'str'}, 'returns': 'int' }
> +
> +##
> # @getfd:
> #
> # Receive a file descriptor via SCM rights and assign it a name
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index f8c0f68..338a0b3 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -869,6 +869,33 @@ Example:
> EQMP
>
> {
> + .name = "passfd",
> + .args_type = "fdname:s",
> + .params = "passfd name",
> + .help = "pass a file descriptor via SCM rights and assign it a name",
> + .mhandler.cmd_new = qmp_marshal_input_passfd,
> + },
> +
> +SQMP
> +passfd
> +------
> +
> +Pass a file descriptor via SCM rights and assign it a name.
> +
> +Arguments:
> +
> +- "fdname": file descriptor name (json-string)
> +
> +Return a json-int with the QEMU file descriptor that was received.
> +
> +Example:
> +
> +-> { "execute": "passfd", "arguments": { "fdname": "fd1" } }
> +<- { "return": 42 }
> +
> +EQMP
> +
> + {
> .name = "getfd",
> .args_type = "fdname:s",
> .params = "getfd name",
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH v2 2/4] qapi: Add passfd QMP command
2012-06-13 19:46 ` Luiz Capitulino
@ 2012-06-13 20:25 ` Corey Bryant
2012-06-13 20:47 ` Eric Blake
0 siblings, 1 reply; 24+ messages in thread
From: Corey Bryant @ 2012-06-13 20:25 UTC (permalink / raw)
To: Luiz Capitulino
Cc: kwolf, aliguori, stefanha, libvir-list, qemu-devel, eblake
On 06/13/2012 03:46 PM, Luiz Capitulino wrote:
> On Fri, 8 Jun 2012 11:42:57 -0400
> Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
>
>> This patch adds the passfd QMP command using the QAPI framework.
>> Like the getfd command, it is used to pass a file descriptor via
>> SCM_RIGHTS. However, the passfd command also returns the received
>> file descriptor, which is a difference in behavior from the getfd
>> command, which returns nothing.
>
> Let's call it pass-fd instead.
Sure, that works for me.
>
> Also, getfd automatically closes a fd if an existing fdname is passed again.
> I don't think this is a good behavior, I think pass-fd should fail instead
> (note that we can't fix getfd though).
>
I agree. It makes sense to fail rather than blindly closing the
existing fd. It can be closed explicitly with closefd if the user wants
it closed.
>>
>> The closefd command can be used to close a file descriptor that was
>> passed with the passfd command.
>>
>> v2:
>> -Introduce new QMP command to pass/return fd (lcapitulino@redhat.com)
>> -Use passfd as command name (berrange@redhat.com)
>>
>> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
>> ---
>> monitor.c | 14 ++++++++++----
>> qapi-schema.json | 15 +++++++++++++++
>> qmp-commands.hx | 27 +++++++++++++++++++++++++++
>> 3 files changed, 52 insertions(+), 4 deletions(-)
>>
>> diff --git a/monitor.c b/monitor.c
>> index 4c53cb6..980a226 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -2182,7 +2182,7 @@ static void do_inject_mce(Monitor *mon, const QDict *qdict)
>> }
>> #endif
>>
>> -void qmp_getfd(const char *fdname, Error **errp)
>> +int64_t qmp_passfd(const char *fdname, Error **errp)
>> {
>> mon_fd_t *monfd;
>> int fd;
>> @@ -2190,13 +2190,13 @@ void qmp_getfd(const char *fdname, Error **errp)
>> fd = qemu_chr_fe_get_msgfd(cur_mon->chr);
>> if (fd == -1) {
>> error_set(errp, QERR_FD_NOT_SUPPLIED);
>> - return;
>> + return -1;
>> }
>>
>> if (qemu_isdigit(fdname[0])) {
>> error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdname",
>> "a name not starting with a digit");
>> - return;
>> + return -1;
>> }
>>
>> QLIST_FOREACH(monfd, &cur_mon->fds, next) {
>> @@ -2206,7 +2206,7 @@ void qmp_getfd(const char *fdname, Error **errp)
>>
>> close(monfd->fd);
>> monfd->fd = fd;
>> - return;
>> + return fd;
>> }
>>
>> monfd = g_malloc0(sizeof(mon_fd_t));
>> @@ -2214,6 +2214,12 @@ void qmp_getfd(const char *fdname, Error **errp)
>> monfd->fd = fd;
>>
>> QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
>> + return fd;
>> +}
>> +
>> +void qmp_getfd(const char *fdname, Error **errp)
>> +{
>> + qmp_passfd(fdname, errp);
>> return;
>> }
>>
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 6be1d90..15da1b8 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -1864,6 +1864,21 @@
>> { 'command': 'netdev_del', 'data': {'id': 'str'} }
>>
>> ##
>> +# @passfd:
>> +#
>> +# Pass a file descriptor via SCM rights and assign it a name
>> +#
>> +# @fdname: file descriptor name
>> +#
>> +# Returns: The QEMU file descriptor that was received
>> +# If file descriptor was not received, FdNotSupplied
>> +# If @fdname is not valid, InvalidParameterType
>> +#
>> +# Since: 1.2.0
>> +##
>> +{ 'command': 'passfd', 'data': {'fdname': 'str'}, 'returns': 'int' }
>> +
>> +##
>> # @getfd:
>> #
>> # Receive a file descriptor via SCM rights and assign it a name
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index f8c0f68..338a0b3 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -869,6 +869,33 @@ Example:
>> EQMP
>>
>> {
>> + .name = "passfd",
>> + .args_type = "fdname:s",
>> + .params = "passfd name",
>> + .help = "pass a file descriptor via SCM rights and assign it a name",
>> + .mhandler.cmd_new = qmp_marshal_input_passfd,
>> + },
>> +
>> +SQMP
>> +passfd
>> +------
>> +
>> +Pass a file descriptor via SCM rights and assign it a name.
>> +
>> +Arguments:
>> +
>> +- "fdname": file descriptor name (json-string)
>> +
>> +Return a json-int with the QEMU file descriptor that was received.
>> +
>> +Example:
>> +
>> +-> { "execute": "passfd", "arguments": { "fdname": "fd1" } }
>> +<- { "return": 42 }
>> +
>> +EQMP
>> +
>> + {
>> .name = "getfd",
>> .args_type = "fdname:s",
>> .params = "getfd name",
>
--
Regards,
Corey
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH v2 2/4] qapi: Add passfd QMP command
2012-06-13 20:25 ` Corey Bryant
@ 2012-06-13 20:47 ` Eric Blake
2012-06-13 22:07 ` Corey Bryant
0 siblings, 1 reply; 24+ messages in thread
From: Eric Blake @ 2012-06-13 20:47 UTC (permalink / raw)
To: Corey Bryant
Cc: kwolf, aliguori, stefanha, libvir-list, qemu-devel,
Luiz Capitulino
[-- Attachment #1: Type: text/plain, Size: 1426 bytes --]
On 06/13/2012 02:25 PM, Corey Bryant wrote:
>> Also, getfd automatically closes a fd if an existing fdname is passed
>> again.
>> I don't think this is a good behavior, I think pass-fd should fail
>> instead
>> (note that we can't fix getfd though).
>>
>
> I agree. It makes sense to fail rather than blindly closing the
> existing fd. It can be closed explicitly with closefd if the user wants
> it closed.
Hmm - what happens if I do 'pass-fd name', learn that qemu is using fd
42, then do 'getfd name'? I silently wipe out fd 42 and replace it with
the new fd passed in by getfd. Which means my use of /dev/fd/42 will
now be broken.
Obviously that means that 'getfd' should NOT be used by any application
using 'pass-fd', and that libvirt should NOT be reusing names (I think
the latter is already true). But I agree that for back-compat we can't
get rid of the current (evil) semantics of a duplicated 'getfd'.
You may also want to mention that when using 'getfd' or 'pass-fd', there
are some commands (like migrate) that use the fd:name protocol, and that
a successful use of one of these commands implicitly closes the named
fd; but that all new uses of /dev/fd/nnn leave the fd open and an
explicit closefd must be used to avoid leaking indefinitely-opened fds
in qemu.
--
Eric Blake eblake@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/4] qapi: Add passfd QMP command
2012-06-13 20:47 ` Eric Blake
@ 2012-06-13 22:07 ` Corey Bryant
2012-06-14 13:28 ` Luiz Capitulino
0 siblings, 1 reply; 24+ messages in thread
From: Corey Bryant @ 2012-06-13 22:07 UTC (permalink / raw)
To: Eric Blake
Cc: kwolf, aliguori, stefanha, libvir-list, qemu-devel,
Luiz Capitulino
On 06/13/2012 04:47 PM, Eric Blake wrote:
> On 06/13/2012 02:25 PM, Corey Bryant wrote:
>
>>> Also, getfd automatically closes a fd if an existing fdname is passed
>>> again.
>>> I don't think this is a good behavior, I think pass-fd should fail
>>> instead
>>> (note that we can't fix getfd though).
>>>
>>
>> I agree. It makes sense to fail rather than blindly closing the
>> existing fd. It can be closed explicitly with closefd if the user wants
>> it closed.
>
> Hmm - what happens if I do 'pass-fd name', learn that qemu is using fd
> 42, then do 'getfd name'? I silently wipe out fd 42 and replace it with
> the new fd passed in by getfd. Which means my use of /dev/fd/42 will
> now be broken.
>
> Obviously that means that 'getfd' should NOT be used by any application
> using 'pass-fd', and that libvirt should NOT be reusing names (I think
> the latter is already true). But I agree that for back-compat we can't
> get rid of the current (evil) semantics of a duplicated 'getfd'.
Yes, users need to be careful and understand how the commands work. I
don't think it's a hard rule that 'getfd' can't be used by an
application that uses 'pass-fd'. If it were, we could put the fds on
separate lists:
struct Monitor {
...
QLIST_HEAD(,mon_fd_t) fds;
+ QLIST_HEAD(,mon_fd_t) pass_fds;
};
But I don't think this is necessary, so I'll plan on documenting them well.
>
> You may also want to mention that when using 'getfd' or 'pass-fd', there
> are some commands (like migrate) that use the fd:name protocol, and that
> a successful use of one of these commands implicitly closes the named
> fd; but that all new uses of /dev/fd/nnn leave the fd open and an
> explicit closefd must be used to avoid leaking indefinitely-opened fds
> in qemu.
>
Ok, I'll mention this too. Thanks.
--
Regards,
Corey
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH v2 2/4] qapi: Add passfd QMP command
2012-06-13 22:07 ` Corey Bryant
@ 2012-06-14 13:28 ` Luiz Capitulino
0 siblings, 0 replies; 24+ messages in thread
From: Luiz Capitulino @ 2012-06-14 13:28 UTC (permalink / raw)
To: Corey Bryant
Cc: kwolf, aliguori, stefanha, libvir-list, qemu-devel, Eric Blake
On Wed, 13 Jun 2012 18:07:43 -0400
Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
>
>
> On 06/13/2012 04:47 PM, Eric Blake wrote:
> > On 06/13/2012 02:25 PM, Corey Bryant wrote:
> >
> >>> Also, getfd automatically closes a fd if an existing fdname is passed
> >>> again.
> >>> I don't think this is a good behavior, I think pass-fd should fail
> >>> instead
> >>> (note that we can't fix getfd though).
> >>>
> >>
> >> I agree. It makes sense to fail rather than blindly closing the
> >> existing fd. It can be closed explicitly with closefd if the user wants
> >> it closed.
> >
> > Hmm - what happens if I do 'pass-fd name', learn that qemu is using fd
> > 42, then do 'getfd name'? I silently wipe out fd 42 and replace it with
> > the new fd passed in by getfd. Which means my use of /dev/fd/42 will
> > now be broken.
> >
> > Obviously that means that 'getfd' should NOT be used by any application
> > using 'pass-fd', and that libvirt should NOT be reusing names (I think
> > the latter is already true). But I agree that for back-compat we can't
> > get rid of the current (evil) semantics of a duplicated 'getfd'.
>
> Yes, users need to be careful and understand how the commands work. I
> don't think it's a hard rule that 'getfd' can't be used by an
> application that uses 'pass-fd'. If it were, we could put the fds on
> separate lists:
>
> struct Monitor {
> ...
> QLIST_HEAD(,mon_fd_t) fds;
> + QLIST_HEAD(,mon_fd_t) pass_fds;
> };
We'd a different closefd command if we do this.
> But I don't think this is necessary, so I'll plan on documenting them well.
Agreed, I don't think this is necessary.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v2 3/4] osdep: Enable qemu_open to dup pre-opened fd
2012-06-08 15:42 [Qemu-devel] [PATCH v2 0/4] file descriptor passing using passfd Corey Bryant
2012-06-08 15:42 ` [Qemu-devel] [PATCH v2 1/4] qapi: Convert getfd and closefd Corey Bryant
2012-06-08 15:42 ` [Qemu-devel] [PATCH v2 2/4] qapi: Add passfd QMP command Corey Bryant
@ 2012-06-08 15:42 ` Corey Bryant
2012-06-08 15:42 ` [Qemu-devel] [PATCH v2 4/4] block: Convert open calls to qemu_open Corey Bryant
` (2 subsequent siblings)
5 siblings, 0 replies; 24+ messages in thread
From: Corey Bryant @ 2012-06-08 15:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, aliguori, stefanha, libvir-list, Corey Bryant, eblake
This patch adds support to qemu_open to dup(X) a pre-opened file
descriptor if the filename is of the format /dev/fd/X.
This can be used when QEMU is restricted from opening files, and
the management application opens files on QEMU's behalf.
v2:
-Get rid of file_open and move dup code to qemu_open (kwolf@redhat.com)
-Use strtol wrapper instead of atoi (kwolf@redhat.com)
Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
---
osdep.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/osdep.c b/osdep.c
index 3e6bada..c17cdcb 100644
--- a/osdep.c
+++ b/osdep.c
@@ -82,6 +82,19 @@ int qemu_open(const char *name, int flags, ...)
int ret;
int mode = 0;
+#ifndef _WIN32
+ const char *p;
+
+ /* Attempt dup of fd for pre-opened file */
+ if (strstart(name, "/dev/fd/", &p)) {
+ ret = qemu_parse_fd(p);
+ if (ret == -1) {
+ return -1;
+ }
+ return dup(ret);
+ }
+#endif
+
if (flags & O_CREAT) {
va_list ap;
--
1.7.10.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH v2 4/4] block: Convert open calls to qemu_open
2012-06-08 15:42 [Qemu-devel] [PATCH v2 0/4] file descriptor passing using passfd Corey Bryant
` (2 preceding siblings ...)
2012-06-08 15:42 ` [Qemu-devel] [PATCH v2 3/4] osdep: Enable qemu_open to dup pre-opened fd Corey Bryant
@ 2012-06-08 15:42 ` Corey Bryant
2012-06-13 10:26 ` Kevin Wolf
2012-06-08 17:10 ` [Qemu-devel] [PATCH v2 0/4] file descriptor passing using passfd Corey Bryant
2012-06-13 10:28 ` Kevin Wolf
5 siblings, 1 reply; 24+ messages in thread
From: Corey Bryant @ 2012-06-08 15:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, aliguori, stefanha, libvir-list, Corey Bryant, eblake
This patch converts all block layer open calls to qemu_open. This
enables all block layer open paths to dup(X) a pre-opened file
descriptor if the filename is of the format /dev/fd/X. This is
useful if QEMU is restricted from opening certain files.
Note that this adds the O_CLOEXEC flag to the changed open paths
when the O_CLOEXEC macro is defined.
v2:
-Convert calls to qemu_open instead of file_open (kwolf@redhat.com)
-Mention introduction of O_CLOEXEC (kwolf@redhat.com)
Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
---
block/raw-posix.c | 18 +++++++++---------
block/raw-win32.c | 4 ++--
block/vdi.c | 5 +++--
block/vmdk.c | 21 +++++++++------------
block/vpc.c | 2 +-
block/vvfat.c | 21 +++++++++++----------
6 files changed, 35 insertions(+), 36 deletions(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 03fcfcc..d8eff2f 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -568,8 +568,8 @@ static int raw_create(const char *filename, QEMUOptionParameter *options)
options++;
}
- fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
- 0644);
+ fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
+ 0644);
if (fd < 0) {
result = -errno;
} else {
@@ -741,7 +741,7 @@ static int hdev_open(BlockDriverState *bs, const char *filename, int flags)
if ( bsdPath[ 0 ] != '\0' ) {
strcat(bsdPath,"s0");
/* some CDs don't have a partition 0 */
- fd = open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE);
+ fd = qemu_open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE);
if (fd < 0) {
bsdPath[strlen(bsdPath)-1] = '1';
} else {
@@ -798,7 +798,7 @@ static int fd_open(BlockDriverState *bs)
#endif
return -EIO;
}
- s->fd = open(bs->filename, s->open_flags & ~O_NONBLOCK);
+ s->fd = qemu_open(bs->filename, s->open_flags & ~O_NONBLOCK);
if (s->fd < 0) {
s->fd_error_time = get_clock();
s->fd_got_error = 1;
@@ -872,7 +872,7 @@ static int hdev_create(const char *filename, QEMUOptionParameter *options)
options++;
}
- fd = open(filename, O_WRONLY | O_BINARY);
+ fd = qemu_open(filename, O_WRONLY | O_BINARY);
if (fd < 0)
return -errno;
@@ -950,7 +950,7 @@ static int floppy_probe_device(const char *filename)
if (strstart(filename, "/dev/fd", NULL))
prio = 50;
- fd = open(filename, O_RDONLY | O_NONBLOCK);
+ fd = qemu_open(filename, O_RDONLY | O_NONBLOCK);
if (fd < 0) {
goto out;
}
@@ -1003,7 +1003,7 @@ static void floppy_eject(BlockDriverState *bs, bool eject_flag)
close(s->fd);
s->fd = -1;
}
- fd = open(bs->filename, s->open_flags | O_NONBLOCK);
+ fd = qemu_open(bs->filename, s->open_flags | O_NONBLOCK);
if (fd >= 0) {
if (ioctl(fd, FDEJECT, 0) < 0)
perror("FDEJECT");
@@ -1053,7 +1053,7 @@ static int cdrom_probe_device(const char *filename)
int prio = 0;
struct stat st;
- fd = open(filename, O_RDONLY | O_NONBLOCK);
+ fd = qemu_open(filename, O_RDONLY | O_NONBLOCK);
if (fd < 0) {
goto out;
}
@@ -1177,7 +1177,7 @@ static int cdrom_reopen(BlockDriverState *bs)
*/
if (s->fd >= 0)
close(s->fd);
- fd = open(bs->filename, s->open_flags, 0644);
+ fd = qemu_open(bs->filename, s->open_flags, 0644);
if (fd < 0) {
s->fd = -1;
return -EIO;
diff --git a/block/raw-win32.c b/block/raw-win32.c
index e4b0b75..8d7838d 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -255,8 +255,8 @@ static int raw_create(const char *filename, QEMUOptionParameter *options)
options++;
}
- fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
- 0644);
+ fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
+ 0644);
if (fd < 0)
return -EIO;
set_sparse(fd);
diff --git a/block/vdi.c b/block/vdi.c
index 119d3c7..6183fdf 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -648,8 +648,9 @@ static int vdi_create(const char *filename, QEMUOptionParameter *options)
options++;
}
- fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
- 0644);
+ fd = qemu_open(filename,
+ O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
+ 0644);
if (fd < 0) {
return -errno;
}
diff --git a/block/vmdk.c b/block/vmdk.c
index 18e9b4c..557dc1b 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1161,10 +1161,9 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
VMDK4Header header;
uint32_t tmp, magic, grains, gd_size, gt_size, gt_count;
- fd = open(
- filename,
- O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
- 0644);
+ fd = qemu_open(filename,
+ O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
+ 0644);
if (fd < 0) {
return -errno;
}
@@ -1484,15 +1483,13 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
(flags & BLOCK_FLAG_COMPAT6 ? 6 : 4),
total_size / (int64_t)(63 * 16 * 512));
if (split || flat) {
- fd = open(
- filename,
- O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
- 0644);
+ fd = qemu_open(filename,
+ O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
+ 0644);
} else {
- fd = open(
- filename,
- O_WRONLY | O_BINARY | O_LARGEFILE,
- 0644);
+ fd = qemu_open(filename,
+ O_WRONLY | O_BINARY | O_LARGEFILE,
+ 0644);
}
if (fd < 0) {
return -errno;
diff --git a/block/vpc.c b/block/vpc.c
index 5cd13d1..60ebf5a 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -678,7 +678,7 @@ static int vpc_create(const char *filename, QEMUOptionParameter *options)
}
/* Create the file */
- fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644);
+ fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644);
if (fd < 0) {
return -EIO;
}
diff --git a/block/vvfat.c b/block/vvfat.c
index 0fd3367..81c3a19 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1152,16 +1152,17 @@ static inline mapping_t* find_mapping_for_cluster(BDRVVVFATState* s,int cluster_
static int open_file(BDRVVVFATState* s,mapping_t* mapping)
{
if(!mapping)
- return -1;
+ return -1;
if(!s->current_mapping ||
- strcmp(s->current_mapping->path,mapping->path)) {
- /* open file */
- int fd = open(mapping->path, O_RDONLY | O_BINARY | O_LARGEFILE);
- if(fd<0)
- return -1;
- vvfat_close_current_file(s);
- s->current_fd = fd;
- s->current_mapping = mapping;
+ strcmp(s->current_mapping->path, mapping->path)) {
+ /* open file */
+ int fd = qemu_open(mapping->path, O_RDONLY | O_BINARY | O_LARGEFILE);
+ if (fd < 0) {
+ return -1;
+ }
+ vvfat_close_current_file(s);
+ s->current_fd = fd;
+ s->current_mapping = mapping;
}
return 0;
}
@@ -2215,7 +2216,7 @@ static int commit_one_file(BDRVVVFATState* s,
for (i = s->cluster_size; i < offset; i += s->cluster_size)
c = modified_fat_get(s, c);
- fd = open(mapping->path, O_RDWR | O_CREAT | O_BINARY, 0666);
+ fd = qemu_open(mapping->path, O_RDWR | O_CREAT | O_BINARY, 0666);
if (fd < 0) {
fprintf(stderr, "Could not open %s... (%s, %d)\n", mapping->path,
strerror(errno), errno);
--
1.7.10.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH v2 4/4] block: Convert open calls to qemu_open
2012-06-08 15:42 ` [Qemu-devel] [PATCH v2 4/4] block: Convert open calls to qemu_open Corey Bryant
@ 2012-06-13 10:26 ` Kevin Wolf
2012-06-13 14:30 ` Corey Bryant
0 siblings, 1 reply; 24+ messages in thread
From: Kevin Wolf @ 2012-06-13 10:26 UTC (permalink / raw)
To: Corey Bryant; +Cc: libvir-list, aliguori, eblake, qemu-devel, stefanha
Am 08.06.2012 17:42, schrieb Corey Bryant:
> This patch converts all block layer open calls to qemu_open. This
> enables all block layer open paths to dup(X) a pre-opened file
> descriptor if the filename is of the format /dev/fd/X. This is
> useful if QEMU is restricted from opening certain files.
>
> Note that this adds the O_CLOEXEC flag to the changed open paths
> when the O_CLOEXEC macro is defined.
>
> v2:
> -Convert calls to qemu_open instead of file_open (kwolf@redhat.com)
> -Mention introduction of O_CLOEXEC (kwolf@redhat.com)
>
> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
> @@ -950,7 +950,7 @@ static int floppy_probe_device(const char *filename)
> if (strstart(filename, "/dev/fd", NULL))
> prio = 50;
Good to have this context here. I think this has to be removed in
another patch or all our file descriptors will become host_floppy...
>
> - fd = open(filename, O_RDONLY | O_NONBLOCK);
> + fd = qemu_open(filename, O_RDONLY | O_NONBLOCK);
> if (fd < 0) {
> goto out;
> }
Kevin
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH v2 4/4] block: Convert open calls to qemu_open
2012-06-13 10:26 ` Kevin Wolf
@ 2012-06-13 14:30 ` Corey Bryant
0 siblings, 0 replies; 24+ messages in thread
From: Corey Bryant @ 2012-06-13 14:30 UTC (permalink / raw)
To: Kevin Wolf; +Cc: libvir-list, aliguori, eblake, qemu-devel, stefanha
On 06/13/2012 06:26 AM, Kevin Wolf wrote:
> Am 08.06.2012 17:42, schrieb Corey Bryant:
>> This patch converts all block layer open calls to qemu_open. This
>> enables all block layer open paths to dup(X) a pre-opened file
>> descriptor if the filename is of the format /dev/fd/X. This is
>> useful if QEMU is restricted from opening certain files.
>>
>> Note that this adds the O_CLOEXEC flag to the changed open paths
>> when the O_CLOEXEC macro is defined.
>>
>> v2:
>> -Convert calls to qemu_open instead of file_open (kwolf@redhat.com)
>> -Mention introduction of O_CLOEXEC (kwolf@redhat.com)
>>
>> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
>
>> @@ -950,7 +950,7 @@ static int floppy_probe_device(const char *filename)
>> if (strstart(filename, "/dev/fd", NULL))
>> prio = 50;
>
> Good to have this context here. I think this has to be removed in
> another patch or all our file descriptors will become host_floppy...
>
Good catch, thanks. I just sent a patch to fix this.
>>
>> - fd = open(filename, O_RDONLY | O_NONBLOCK);
>> + fd = qemu_open(filename, O_RDONLY | O_NONBLOCK);
>> if (fd < 0) {
>> goto out;
>> }
>
> Kevin
>
--
Regards,
Corey
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/4] file descriptor passing using passfd
2012-06-08 15:42 [Qemu-devel] [PATCH v2 0/4] file descriptor passing using passfd Corey Bryant
` (3 preceding siblings ...)
2012-06-08 15:42 ` [Qemu-devel] [PATCH v2 4/4] block: Convert open calls to qemu_open Corey Bryant
@ 2012-06-08 17:10 ` Corey Bryant
2012-06-13 10:28 ` Kevin Wolf
5 siblings, 0 replies; 24+ messages in thread
From: Corey Bryant @ 2012-06-08 17:10 UTC (permalink / raw)
To: Corey Bryant; +Cc: kwolf, aliguori, stefanha, libvir-list, qemu-devel, eblake
Please review this patch series if you could. I apologize for sending
it more than once. Thanks!
--
Regards,
Corey
On 06/08/2012 11:42 AM, Corey Bryant wrote:
> libvirt's sVirt security driver provides SELinux MAC isolation for
> Qemu guest processes and their corresponding image files. In other
> words, sVirt uses SELinux to prevent a QEMU process from opening
> files that do not belong to it.
>
> sVirt provides this support by labeling guests and resources with
> security labels that are stored in file system extended attributes.
> Some file systems, such as NFS, do not support the extended
> attribute security namespace, and therefore cannot support sVirt
> isolation.
>
> A solution to this problem is to provide fd passing support, where
> libvirt opens files and passes file descriptors to QEMU. This,
> along with SELinux policy to prevent QEMU from opening files, can
> provide image file isolation for NFS files stored on the same NFS
> mount.
>
> This patch series adds the passfd QMP monitor command, which allows
> an fd to be passed via SCM_RIGHTS, and returns the received file
> descriptor. Support is also added to the block layer to allow QEMU
> to dup the fd when the filename is of the /dev/fd/X format. This
> is useful if MAC policy prevents QEMU from opening specific types
> of files.
>
> One nice thing about this approach is that no new SELinux policy is
> required to prevent open of NFS files (files with type nfs_t). The
> virt_use_nfs boolean type simply needs to be set to false, and open
> will be prevented (and dup will be allowed). For example:
>
> # setsebool virt_use_nfs 0
> # getsebool virt_use_nfs
> virt_use_nfs --> off
>
> Corey Bryant (4):
> qapi: Convert getfd and closefd
> qapi: Add passfd QMP command
> osdep: Enable qemu_open to dup pre-opened fd
> block: Convert open calls to qemu_open
>
> block/raw-posix.c | 18 +++++++++---------
> block/raw-win32.c | 4 ++--
> block/vdi.c | 5 +++--
> block/vmdk.c | 21 +++++++++------------
> block/vpc.c | 2 +-
> block/vvfat.c | 21 +++++++++++----------
> hmp-commands.hx | 6 ++----
> hmp.c | 18 ++++++++++++++++++
> hmp.h | 2 ++
> monitor.c | 36 ++++++++++++++++++++----------------
> osdep.c | 13 +++++++++++++
> qapi-schema.json | 44 ++++++++++++++++++++++++++++++++++++++++++++
> qmp-commands.hx | 33 +++++++++++++++++++++++++++++----
> 13 files changed, 163 insertions(+), 60 deletions(-)
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH v2 0/4] file descriptor passing using passfd
2012-06-08 15:42 [Qemu-devel] [PATCH v2 0/4] file descriptor passing using passfd Corey Bryant
` (4 preceding siblings ...)
2012-06-08 17:10 ` [Qemu-devel] [PATCH v2 0/4] file descriptor passing using passfd Corey Bryant
@ 2012-06-13 10:28 ` Kevin Wolf
2012-06-13 14:31 ` Corey Bryant
5 siblings, 1 reply; 24+ messages in thread
From: Kevin Wolf @ 2012-06-13 10:28 UTC (permalink / raw)
To: Corey Bryant
Cc: aliguori, stefanha, libvir-list, qemu-devel, Luiz Capitulino,
eblake
Am 08.06.2012 17:42, schrieb Corey Bryant:
> libvirt's sVirt security driver provides SELinux MAC isolation for
> Qemu guest processes and their corresponding image files. In other
> words, sVirt uses SELinux to prevent a QEMU process from opening
> files that do not belong to it.
>
> sVirt provides this support by labeling guests and resources with
> security labels that are stored in file system extended attributes.
> Some file systems, such as NFS, do not support the extended
> attribute security namespace, and therefore cannot support sVirt
> isolation.
>
> A solution to this problem is to provide fd passing support, where
> libvirt opens files and passes file descriptors to QEMU. This,
> along with SELinux policy to prevent QEMU from opening files, can
> provide image file isolation for NFS files stored on the same NFS
> mount.
>
> This patch series adds the passfd QMP monitor command, which allows
> an fd to be passed via SCM_RIGHTS, and returns the received file
> descriptor. Support is also added to the block layer to allow QEMU
> to dup the fd when the filename is of the /dev/fd/X format. This
> is useful if MAC policy prevents QEMU from opening specific types
> of files.
>
> One nice thing about this approach is that no new SELinux policy is
> required to prevent open of NFS files (files with type nfs_t). The
> virt_use_nfs boolean type simply needs to be set to false, and open
> will be prevented (and dup will be allowed). For example:
>
> # setsebool virt_use_nfs 0
> # getsebool virt_use_nfs
> virt_use_nfs --> off
>
> Corey Bryant (4):
> qapi: Convert getfd and closefd
> qapi: Add passfd QMP command
> osdep: Enable qemu_open to dup pre-opened fd
> block: Convert open calls to qemu_open
>
> block/raw-posix.c | 18 +++++++++---------
> block/raw-win32.c | 4 ++--
> block/vdi.c | 5 +++--
> block/vmdk.c | 21 +++++++++------------
> block/vpc.c | 2 +-
> block/vvfat.c | 21 +++++++++++----------
> hmp-commands.hx | 6 ++----
> hmp.c | 18 ++++++++++++++++++
> hmp.h | 2 ++
> monitor.c | 36 ++++++++++++++++++++----------------
> osdep.c | 13 +++++++++++++
> qapi-schema.json | 44 ++++++++++++++++++++++++++++++++++++++++++++
> qmp-commands.hx | 33 +++++++++++++++++++++++++++++----
> 13 files changed, 163 insertions(+), 60 deletions(-)
Looks good to me. If Luiz is okay with the QMP part, I'm going to apply
this to the block branch.
Corey, please make sure to check the host_floppy problem and send a
patch if necessary.
Kevin
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH v2 0/4] file descriptor passing using passfd
2012-06-13 10:28 ` Kevin Wolf
@ 2012-06-13 14:31 ` Corey Bryant
0 siblings, 0 replies; 24+ messages in thread
From: Corey Bryant @ 2012-06-13 14:31 UTC (permalink / raw)
To: Kevin Wolf
Cc: aliguori, stefanha, libvir-list, qemu-devel, Luiz Capitulino,
eblake
On 06/13/2012 06:28 AM, Kevin Wolf wrote:
> Am 08.06.2012 17:42, schrieb Corey Bryant:
>> libvirt's sVirt security driver provides SELinux MAC isolation for
>> Qemu guest processes and their corresponding image files. In other
>> words, sVirt uses SELinux to prevent a QEMU process from opening
>> files that do not belong to it.
>>
>> sVirt provides this support by labeling guests and resources with
>> security labels that are stored in file system extended attributes.
>> Some file systems, such as NFS, do not support the extended
>> attribute security namespace, and therefore cannot support sVirt
>> isolation.
>>
>> A solution to this problem is to provide fd passing support, where
>> libvirt opens files and passes file descriptors to QEMU. This,
>> along with SELinux policy to prevent QEMU from opening files, can
>> provide image file isolation for NFS files stored on the same NFS
>> mount.
>>
>> This patch series adds the passfd QMP monitor command, which allows
>> an fd to be passed via SCM_RIGHTS, and returns the received file
>> descriptor. Support is also added to the block layer to allow QEMU
>> to dup the fd when the filename is of the /dev/fd/X format. This
>> is useful if MAC policy prevents QEMU from opening specific types
>> of files.
>>
>> One nice thing about this approach is that no new SELinux policy is
>> required to prevent open of NFS files (files with type nfs_t). The
>> virt_use_nfs boolean type simply needs to be set to false, and open
>> will be prevented (and dup will be allowed). For example:
>>
>> # setsebool virt_use_nfs 0
>> # getsebool virt_use_nfs
>> virt_use_nfs --> off
>>
>> Corey Bryant (4):
>> qapi: Convert getfd and closefd
>> qapi: Add passfd QMP command
>> osdep: Enable qemu_open to dup pre-opened fd
>> block: Convert open calls to qemu_open
>>
>> block/raw-posix.c | 18 +++++++++---------
>> block/raw-win32.c | 4 ++--
>> block/vdi.c | 5 +++--
>> block/vmdk.c | 21 +++++++++------------
>> block/vpc.c | 2 +-
>> block/vvfat.c | 21 +++++++++++----------
>> hmp-commands.hx | 6 ++----
>> hmp.c | 18 ++++++++++++++++++
>> hmp.h | 2 ++
>> monitor.c | 36 ++++++++++++++++++++----------------
>> osdep.c | 13 +++++++++++++
>> qapi-schema.json | 44 ++++++++++++++++++++++++++++++++++++++++++++
>> qmp-commands.hx | 33 +++++++++++++++++++++++++++++----
>> 13 files changed, 163 insertions(+), 60 deletions(-)
>
> Looks good to me. If Luiz is okay with the QMP part, I'm going to apply
> this to the block branch.
>
> Corey, please make sure to check the host_floppy problem and send a
> patch if necessary.
>
> Kevin
>
Thanks! I just sent a patch for the host_floppy issue.
--
Regards,
Corey
^ permalink raw reply [flat|nested] 24+ messages in thread