On Wed, Aug 18, 2010 at 01:35:29PM +0200, soren@xxxxxxxxxxx wrote:
> From: Soren Hansen <soren@xxxxxxxxxxx>
>
> Like that comment suggested, we just open the file and pass the file
> descriptor to uml. The input "stream" is set to "null", since I couldn't
> find any useful way to actually use a file for input for a chardev and
> this also mimics what e.g. QEmu does internally.
Yep, this looks fine.
> Signed-off-by: Soren Hansen <soren@xxxxxxxxxxx>
> ---
> src/uml/uml_conf.c | 30 +++++++++++++++++++++++++-----
> src/uml/uml_conf.h | 1 +
> src/uml/uml_driver.c | 2 +-
> 3 files changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c
> index bc8cbce..659f881 100644
> --- a/src/uml/uml_conf.c
> +++ b/src/uml/uml_conf.c
> @@ -297,7 +297,8 @@ error:
>
> static char *
> umlBuildCommandLineChr(virDomainChrDefPtr def,
> - const char *dev)
> + const char *dev,
> + fd_set *keepfd)
> {
> char *ret = NULL;
>
> @@ -346,8 +347,26 @@ umlBuildCommandLineChr(virDomainChrDefPtr def,
> break;
>
> case VIR_DOMAIN_CHR_TYPE_FILE:
> - case VIR_DOMAIN_CHR_TYPE_PIPE:
> - /* XXX could open the file/pipe & just pass the FDs */
> + {
> + int fd_out;
> +
> + if ((fd_out = open(def->data.file.path,
> + O_WRONLY | O_APPEND | O_CREAT, 0660)) < 0) {
> + virReportSystemError(errno,
> + _("failed to open chardev file: %s"),
> + def->data.file.path);
> + return NULL;
> + }
> + if (virAsprintf(&ret, "%s%d=null,fd:%d", dev, def->target.port, fd_out) < 0) {
> + virReportOOMError();
> + close(fd_out);
> + return NULL;
> + }
> + FD_SET(fd_out, keepfd);
> + }
> + break;
> + case VIR_DOMAIN_CHR_TYPE_PIPE:
> + /* XXX could open the pipe & just pass the FDs */
Any reason not to let the code deal with PIPE too ? It seems
like the code should work equally well for both PIPE & FILE.
(well drop the O_CREATE|O_APPEND - assume the user has got
the pipe pre-created with 'mkfifo'.
> case VIR_DOMAIN_CHR_TYPE_VC:
> case VIR_DOMAIN_CHR_TYPE_UDP:
> @@ -393,6 +412,7 @@ static char *umlNextArg(char *args)
> int umlBuildCommandLine(virConnectPtr conn,
> struct uml_driver *driver ATTRIBUTE_UNUSED,
> virDomainObjPtr vm,
> + fd_set *keepfd,
> const char ***retargv,
> const char ***retenv)
> {
> @@ -515,7 +535,7 @@ int umlBuildCommandLine(virConnectPtr conn,
> for (i = 0 ; i < UML_MAX_CHAR_DEVICE ; i++) {
> char *ret;
> if (i == 0 && vm->def->console)
> - ret = umlBuildCommandLineChr(vm->def->console, "con");
> + ret = umlBuildCommandLineChr(vm->def->console, "con", keepfd);
> else
> if (virAsprintf(&ret, "con%d=none", i) < 0)
> goto no_memory;
> @@ -529,7 +549,7 @@ int umlBuildCommandLine(virConnectPtr conn,
> if (vm->def->serials[j]->target.port == i)
> chr = vm->def->serials[j];
> if (chr)
> - ret = umlBuildCommandLineChr(chr, "ssl");
> + ret = umlBuildCommandLineChr(chr, "ssl", keepfd);
> else
> if (virAsprintf(&ret, "ssl%d=none", i) < 0)
> goto no_memory;
> diff --git a/src/uml/uml_conf.h b/src/uml/uml_conf.h
> index b33acc8..d8b2349 100644
> --- a/src/uml/uml_conf.h
> +++ b/src/uml/uml_conf.h
> @@ -71,6 +71,7 @@ virCapsPtr umlCapsInit (void);
> int umlBuildCommandLine (virConnectPtr conn,
> struct uml_driver *driver,
> virDomainObjPtr dom,
> + fd_set *keepfd,
> const char ***retargv,
> const char ***retenv);
>
> diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
> index 04493ba..73f77f8 100644
> --- a/src/uml/uml_driver.c
> +++ b/src/uml/uml_driver.c
> @@ -871,7 +871,7 @@ static int umlStartVMDaemon(virConnectPtr conn,
> return -1;
> }
>
> - if (umlBuildCommandLine(conn, driver, vm,
> + if (umlBuildCommandLine(conn, driver, vm, &keepfd,
> &argv, &progenv) < 0) {
> close(logfd);
> umlCleanupTapDevices(conn, vm);
I think this leaks file descriptors in libvirtd. There's no existing
code which ever closes the FDs in keepfd after spawning UML later
on in the function.
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list