* [Qemu-devel] [PATCH v2 1/4] qapi: Convert getfd and closefd
2012-06-08 14:49 [Qemu-devel] [PATCH v2 0/4] file descriptor passing using passfd Corey Bryant
@ 2012-06-08 14:49 ` Corey Bryant
0 siblings, 0 replies; 24+ messages in thread
From: Corey Bryant @ 2012-06-08 14:49 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
* [Qemu-devel] [PATCH v2 1/4] qapi: Convert getfd and closefd
2012-06-08 14:53 Corey Bryant
@ 2012-06-08 14:53 ` Corey Bryant
0 siblings, 0 replies; 24+ messages in thread
From: Corey Bryant @ 2012-06-08 14:53 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
* [Qemu-devel] [PATCH v2 0/4] file descriptor passing using passfd
@ 2012-06-08 15:42 Corey Bryant
2012-06-08 15:42 ` [Qemu-devel] [PATCH v2 1/4] qapi: Convert getfd and closefd Corey Bryant
` (5 more replies)
0 siblings, 6 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
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(-)
--
1.7.10.2
^ permalink raw reply [flat|nested] 24+ messages in thread
* [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
* [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
* [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 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 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 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 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-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
* 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-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 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 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-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 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 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 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 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
* 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
end of thread, other threads:[~2012-06-14 13:28 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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-13 19:41 ` Luiz Capitulino
2012-06-13 20:10 ` Corey Bryant
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
2012-06-13 21:43 ` Corey Bryant
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
2012-06-13 20:47 ` Eric Blake
2012-06-13 22:07 ` Corey Bryant
2012-06-14 13:28 ` Luiz Capitulino
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 ` [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
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
-- strict thread matches above, loose matches on Subject: below --
2012-06-08 14:53 Corey Bryant
2012-06-08 14:53 ` [Qemu-devel] [PATCH v2 1/4] qapi: Convert getfd and closefd Corey Bryant
2012-06-08 14:49 [Qemu-devel] [PATCH v2 0/4] file descriptor passing using passfd Corey Bryant
2012-06-08 14:49 ` [Qemu-devel] [PATCH v2 1/4] qapi: Convert getfd and closefd Corey Bryant
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).