From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:60528) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TQJb5-0005PE-An for qemu-devel@nongnu.org; Mon, 22 Oct 2012 11:07:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TQJay-0001pY-P2 for qemu-devel@nongnu.org; Mon, 22 Oct 2012 11:07:19 -0400 Received: from e9.ny.us.ibm.com ([32.97.182.139]:57903) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TQJay-0001pO-Ju for qemu-devel@nongnu.org; Mon, 22 Oct 2012 11:07:12 -0400 Received: from /spool/local by e9.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 22 Oct 2012 11:07:11 -0400 Received: from d01relay03.pok.ibm.com (d01relay03.pok.ibm.com [9.56.227.235]) by d01dlp01.pok.ibm.com (Postfix) with ESMTP id A50B138C8059 for ; Mon, 22 Oct 2012 11:07:06 -0400 (EDT) Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay03.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q9MF73eT291632 for ; Mon, 22 Oct 2012 11:07:04 -0400 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q9MF72KQ000585 for ; Mon, 22 Oct 2012 13:07:02 -0200 Message-ID: <50856113.6050706@linux.vnet.ibm.com> Date: Mon, 22 Oct 2012 11:06:59 -0400 From: Corey Bryant MIME-Version: 1.0 References: <1350587974-6378-5-git-send-email-coreyb@linux.vnet.ibm.com> <1350916563-15700-1-git-send-email-kwolf@redhat.com> In-Reply-To: <1350916563-15700-1-git-send-email-kwolf@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5] qemu-config: Add new -add-fd command line option List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org On 10/22/2012 10:36 AM, Kevin Wolf wrote: > From: Corey Bryant > > This option can be used for passing file descriptors on the > command line. It mirrors the existing add-fd QMP command which > allows an fd to be passed to QEMU via SCM_RIGHTS and added to an > fd set. > > This can be combined with commands such as -drive to link file > descriptors in an fd set to a drive: > > qemu-kvm -add-fd fd=3,set=2,opaque="rdwr:/path/to/file" > -add-fd fd=4,set=2,opaque="rdonly:/path/to/file" > -drive file=/dev/fdset/2,index=0,media=disk > > This example adds dups of fds 3 and 4, and the accompanying opaque > strings to the fd set with ID=2. qemu_open() already knows how > to handle a filename of this format. qemu_open() searches the > corresponding fd set for an fd and when it finds a match, QEMU > goes on to use a dup of that fd just like it would have used an > fd that it opened itself. > > Signed-off-by: Corey Bryant > Reviewed-by: Eric Blake > Signed-off-by: Kevin Wolf > --- > > Sorry, Corey, hope you're okay with me taking over your patch... Your patch was > against the unmodified version while I already did some changes after the v4 > review, so it didn't apply. > That's fine. Thanks for the hand. > This version just completely disables fd passing on Windows as I don't think > it works there anyway. Gives you a nice error message instead of a silently > ignored -add-fd option. > > Also added the missing break for case QEMU_OPTION_add_fd. > > qemu-config.c | 22 +++++++++++++ > qemu-options.hx | 36 +++++++++++++++++++++ > vl.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 152 insertions(+), 0 deletions(-) > > diff --git a/qemu-config.c b/qemu-config.c > index cd1ec21..601237d 100644 > --- a/qemu-config.c > +++ b/qemu-config.c > @@ -653,6 +653,27 @@ QemuOptsList qemu_boot_opts = { > }, > }; > > +static QemuOptsList qemu_add_fd_opts = { > + .name = "add-fd", > + .head = QTAILQ_HEAD_INITIALIZER(qemu_add_fd_opts.head), > + .desc = { > + { > + .name = "fd", > + .type = QEMU_OPT_NUMBER, > + .help = "file descriptor of which a duplicate is added to fd set", > + },{ > + .name = "set", > + .type = QEMU_OPT_NUMBER, > + .help = "ID of the fd set to add fd to", > + },{ > + .name = "opaque", > + .type = QEMU_OPT_STRING, > + .help = "free-form string used to describe fd", > + }, > + { /* end of list */ } > + }, > +}; > + > static QemuOptsList *vm_config_groups[32] = { > &qemu_drive_opts, > &qemu_chardev_opts, > @@ -669,6 +690,7 @@ static QemuOptsList *vm_config_groups[32] = { > &qemu_boot_opts, > &qemu_iscsi_opts, > &qemu_sandbox_opts, > + &qemu_add_fd_opts, > NULL, > }; > > diff --git a/qemu-options.hx b/qemu-options.hx > index 46f0539..a67a255 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -253,6 +253,14 @@ qemu-system-i386 -drive file=file,index=2,media=disk > qemu-system-i386 -drive file=file,index=3,media=disk > @end example > > +You can open an image using pre-opened file descriptors from an fd set: > +@example > +qemu-system-i386 > +-add-fd fd=3,set=2,opaque="rdwr:/path/to/file" > +-add-fd fd=4,set=2,opaque="rdonly:/path/to/file" > +-drive file=/dev/fdset/2,index=0,media=disk > +@end example > + > You can connect a CDROM to the slave of ide0: > @example > qemu-system-i386 -drive file=file,if=ide,index=1,media=cdrom > @@ -285,6 +293,34 @@ qemu-system-i386 -hda a -hdb b > @end example > ETEXI > > +DEF("add-fd", HAS_ARG, QEMU_OPTION_add_fd, > + "-add-fd fd=fd,set=set[,opaque=opaque]\n" > + " Add 'fd' to fd 'set'\n", QEMU_ARCH_ALL) > +STEXI > +@item -add-fd fd=@var{fd},set=@var{set}[,opaque=@var{opaque}] > +@findex -add-fd > + > +Add a file descriptor to an fd set. Valid options are: > + > +@table @option > +@item fd=@var{fd} > +This option defines the file descriptor of which a duplicate is added to fd set. > +The file descriptor cannot be stdin, stdout, or stderr. > +@item set=@var{set} > +This option defines the ID of the fd set to add the file descriptor to. > +@item opaque=@var{opaque} > +This option defines a free-form string that can be used to describe @var{fd}. > +@end table > + > +You can open an image using pre-opened file descriptors from an fd set: > +@example > +qemu-system-i386 > +-add-fd fd=3,set=2,opaque="rdwr:/path/to/file" > +-add-fd fd=4,set=2,opaque="rdonly:/path/to/file" > +-drive file=/dev/fdset/2,index=0,media=disk > +@end example > +ETEXI > + > DEF("set", HAS_ARG, QEMU_OPTION_set, > "-set group.id.arg=value\n" > " set parameter for item of type \n" > diff --git a/vl.c b/vl.c > index ee3c43a..b870caf 100644 > --- a/vl.c > +++ b/vl.c > @@ -790,6 +790,78 @@ static int parse_sandbox(QemuOpts *opts, void *opaque) > return 0; > } > > +#ifndef _WIN32 > +static int parse_add_fd(QemuOpts *opts, void *opaque) > +{ > + int fd, dupfd, flags; > + int64_t fdset_id; > + const char *fd_opaque = NULL; > + > + fd = qemu_opt_get_number(opts, "fd", -1); > + fdset_id = qemu_opt_get_number(opts, "set", -1); > + fd_opaque = qemu_opt_get(opts, "opaque"); > + > + if (fd < 0) { > + qerror_report(ERROR_CLASS_GENERIC_ERROR, > + "fd option is required and must be non-negative"); > + return -1; > + } > + > + if (fd <= STDERR_FILENO) { > + qerror_report(ERROR_CLASS_GENERIC_ERROR, > + "fd cannot be a standard I/O stream"); > + return -1; > + } > + > + /* > + * All fds inherited across exec() necessarily have FD_CLOEXEC > + * clear, while qemu sets FD_CLOEXEC on all other fds used internally. > + */ > + flags = fcntl(fd, F_GETFD); > + if (flags == -1 || (flags & FD_CLOEXEC)) { > + qerror_report(ERROR_CLASS_GENERIC_ERROR, > + "fd is not valid or already in use"); > + return -1; > + } > + > + if (fdset_id < 0) { > + qerror_report(ERROR_CLASS_GENERIC_ERROR, > + "set option is required and must be non-negative"); > + return -1; > + } > + > +#ifdef F_DUPFD_CLOEXEC > + dupfd = fcntl(fd, F_DUPFD_CLOEXEC, 0); > +#else > + dupfd = dup(fd); > + if (dupfd != -1) { > + qemu_set_cloexec(dupfd); > + } > +#endif > + if (dupfd == -1) { > + qerror_report(ERROR_CLASS_GENERIC_ERROR, > + "Error duplicating fd: %s", strerror(errno)); > + return -1; > + } > + > + /* add the duplicate fd, and optionally the opaque string, to the fd set */ > + monitor_fdset_add_fd(dupfd, true, fdset_id, fd_opaque ? true : false, > + fd_opaque, NULL); > + > + return 0; > +} > + > +static int cleanup_add_fd(QemuOpts *opts, void *opaque) > +{ > + int fd; > + > + fd = qemu_opt_get_number(opts, "fd", -1); > + close(fd); > + > + return 0; > +} > +#endif > + > /***********************************************************/ > /* QEMU Block devices */ > > @@ -3309,6 +3381,18 @@ int main(int argc, char **argv, char **envp) > exit(0); > } > break; > + case QEMU_OPTION_add_fd: > +#ifndef _WIN32 > + opts = qemu_opts_parse(qemu_find_opts("add-fd"), optarg, 0); > + if (!opts) { > + exit(0); > + } > +#else > + error_report("File descriptor passing is disabled on this " > + "platform"); > + exit(1); > +#endif > + break; > default: > os_parse_cmd_args(popt->index, optarg); > } > @@ -3320,6 +3404,16 @@ int main(int argc, char **argv, char **envp) > exit(1); > } > > +#ifndef _WIN32 > + if (qemu_opts_foreach(qemu_find_opts("add-fd"), parse_add_fd, NULL, 1)) { > + exit(1); > + } > + > + if (qemu_opts_foreach(qemu_find_opts("add-fd"), cleanup_add_fd, NULL, 1)) { > + exit(1); > + } > +#endif > + > if (machine == NULL) { > fprintf(stderr, "No machine found.\n"); > exit(1); > This looks good to me. -- Regards, Corey Bryant