Re: [libvirt] [PATCH] Allow chardev of type 'file' for UML domains.
2010/8/23 Daniel P. Berrange berra...@redhat.com: On Mon, Aug 23, 2010 at 01:25:50PM +0200, Soren Hansen wrote: Like the 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. Signed-off-by: Soren Hansen so...@linux2go.dk --- src/uml/uml_conf.c | 31 ++- src/uml/uml_conf.h | 1 + src/uml/uml_driver.c | 10 +- 3 files changed, 36 insertions(+), 6 deletions(-) ACK, looks good now. I applied and pushed this patch. Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Allow chardev of type 'file' for UML domains.
Like the 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. Signed-off-by: Soren Hansen so...@linux2go.dk --- src/uml/uml_conf.c | 31 ++- src/uml/uml_conf.h |1 + src/uml/uml_driver.c | 12 +++- 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index 42193e4..65b06c5 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -295,7 +295,8 @@ error: static char * umlBuildCommandLineChr(virDomainChrDefPtr def, - const char *dev) + const char *dev, + fd_set *keepfd) { char *ret = NULL; @@ -344,8 +345,27 @@ 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. Be wary of + * the effects of blocking I/O, though. */ case VIR_DOMAIN_CHR_TYPE_VC: case VIR_DOMAIN_CHR_TYPE_UDP: @@ -391,6 +411,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) { @@ -513,7 +534,7 @@ int umlBuildCommandLine(virConnectPtr conn, for (i = 0 ; i UML_MAX_CHAR_DEVICE ; i++) { char *ret = NULL; if (i == 0 vm-def-console) -ret = umlBuildCommandLineChr(vm-def-console, con); +ret = umlBuildCommandLineChr(vm-def-console, con, keepfd); if (!ret) if (virAsprintf(ret, con%d=none, i) 0) goto no_memory; @@ -527,7 +548,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); if (!ret) 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 9cad7f1..e926a9f 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -811,6 +811,7 @@ static int umlStartVMDaemon(virConnectPtr conn, char *logfile; int logfd = -1; struct stat sb; +int openmax; fd_set keepfd; char ebuf[1024]; umlDomainObjPrivatePtr priv = vm-privateData; @@ -869,7 +870,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); @@ -908,6 +909,15 @@ static int umlStartVMDaemon(virConnectPtr conn, NULL, NULL, NULL); close(logfd); +/* + * At the moment, the only thing that populates keepfd is + * umlBuildCommandLineChr. We want to close every fd it opens. + */ +openmax = sysconf (_SC_OPEN_MAX); +for (i = 0; i openmax; i++) +if (FD_ISSET(i, keepfd)) +close(i); + for (i = 0 ; argv[i] ; i++) VIR_FREE(argv[i]); VIR_FREE(argv); -- 1.7.0.4 -- libvir-list mailing list libvir-list@redhat.com
Re: [libvirt] [PATCH] Allow chardev of type 'file' for UML domains.
On Mon, Aug 23, 2010 at 12:19:51PM +0200, Soren Hansen wrote: Like the 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. Signed-off-by: Soren Hansen so...@linux2go.dk --- src/uml/uml_conf.c | 31 ++- src/uml/uml_conf.h |1 + src/uml/uml_driver.c | 12 +++- 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 9cad7f1..e926a9f 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -811,6 +811,7 @@ static int umlStartVMDaemon(virConnectPtr conn, char *logfile; int logfd = -1; struct stat sb; +int openmax; fd_set keepfd; char ebuf[1024]; umlDomainObjPrivatePtr priv = vm-privateData; @@ -869,7 +870,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); @@ -908,6 +909,15 @@ static int umlStartVMDaemon(virConnectPtr conn, NULL, NULL, NULL); close(logfd); +/* + * At the moment, the only thing that populates keepfd is + * umlBuildCommandLineChr. We want to close every fd it opens. + */ +openmax = sysconf (_SC_OPEN_MAX); +for (i = 0; i openmax; i++) +if (FD_ISSET(i, keepfd)) +close(i); + Unfortunately fdset is one of those limited types that can't represent all possible values. So you need to use FD_SETSIZE instead of _SC_OPEN_MAX here 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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Allow chardev of type 'file' for UML domains.
On 23-08-2010 12:42, Daniel P. Berrange wrote: +/* + * At the moment, the only thing that populates keepfd is + * umlBuildCommandLineChr. We want to close every fd it opens. + */ +openmax = sysconf (_SC_OPEN_MAX); +for (i = 0; i openmax; i++) +if (FD_ISSET(i, keepfd)) +close(i); + Unfortunately fdset is one of those limited types that can't represent all possible values. So you need to use FD_SETSIZE instead of _SC_OPEN_MAX here Ok, I'll fix that up, but just so that I understand: Your concern is that there might be an open file descriptor between FD_SETSIZE and _SC_OPEN_MAX that we don't want to close? -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Allow chardev of type 'file' for UML domains.
On Mon, Aug 23, 2010 at 12:59:16PM +0200, Soren Hansen wrote: On 23-08-2010 12:42, Daniel P. Berrange wrote: +/* + * At the moment, the only thing that populates keepfd is + * umlBuildCommandLineChr. We want to close every fd it opens. + */ +openmax = sysconf (_SC_OPEN_MAX); +for (i = 0; i openmax; i++) +if (FD_ISSET(i, keepfd)) +close(i); + Unfortunately fdset is one of those limited types that can't represent all possible values. So you need to use FD_SETSIZE instead of _SC_OPEN_MAX here Ok, I'll fix that up, but just so that I understand: Your concern is that there might be an open file descriptor between FD_SETSIZE and _SC_OPEN_MAX that we don't want to close? No, its that if you try to run FD_ISSET for i FD_SETSIZE, you'll likely have an array overflow / out of bounds, so just stop at FD_SETSIZE. When we switch to the new virCommandPtr apis we'll remove this limitation. 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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Allow chardev of type 'file' for UML domains.
On 23-08-2010 13:04, Daniel P. Berrange wrote: Ok, I'll fix that up, but just so that I understand: Your concern is that there might be an open file descriptor between FD_SETSIZE and _SC_OPEN_MAX that we don't want to close? No, its that if you try to run FD_ISSET for i FD_SETSIZE, you'll likely have an array overflow / out of bounds, so just stop at FD_SETSIZE. Oh, of course. How silly of me. Thanks :) -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Allow chardev of type 'file' for UML domains.
Like the 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. Signed-off-by: Soren Hansen so...@linux2go.dk --- src/uml/uml_conf.c | 31 ++- src/uml/uml_conf.h |1 + src/uml/uml_driver.c | 10 +- 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index 42193e4..65b06c5 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -295,7 +295,8 @@ error: static char * umlBuildCommandLineChr(virDomainChrDefPtr def, - const char *dev) + const char *dev, + fd_set *keepfd) { char *ret = NULL; @@ -344,8 +345,27 @@ 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. Be wary of + * the effects of blocking I/O, though. */ case VIR_DOMAIN_CHR_TYPE_VC: case VIR_DOMAIN_CHR_TYPE_UDP: @@ -391,6 +411,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) { @@ -513,7 +534,7 @@ int umlBuildCommandLine(virConnectPtr conn, for (i = 0 ; i UML_MAX_CHAR_DEVICE ; i++) { char *ret = NULL; if (i == 0 vm-def-console) -ret = umlBuildCommandLineChr(vm-def-console, con); +ret = umlBuildCommandLineChr(vm-def-console, con, keepfd); if (!ret) if (virAsprintf(ret, con%d=none, i) 0) goto no_memory; @@ -527,7 +548,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); if (!ret) 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 9cad7f1..6582d95 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -869,7 +869,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); @@ -908,6 +908,14 @@ static int umlStartVMDaemon(virConnectPtr conn, NULL, NULL, NULL); close(logfd); +/* + * At the moment, the only thing that populates keepfd is + * umlBuildCommandLineChr. We want to close every fd it opens. + */ +for (i = 0; i FD_SETSIZE; i++) +if (FD_ISSET(i, keepfd)) +close(i); + for (i = 0 ; argv[i] ; i++) VIR_FREE(argv[i]); VIR_FREE(argv); -- 1.7.0.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Allow chardev of type 'file' for UML domains.
On Mon, Aug 23, 2010 at 01:25:50PM +0200, Soren Hansen wrote: Like the 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. Signed-off-by: Soren Hansen so...@linux2go.dk --- src/uml/uml_conf.c | 31 ++- src/uml/uml_conf.h |1 + src/uml/uml_driver.c | 10 +- 3 files changed, 36 insertions(+), 6 deletions(-) ACK, looks good now. 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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Allow chardev of type 'file' for UML domains.
On Wed, Aug 18, 2010 at 01:35:29PM +0200, so...@linux2go.dk wrote: From: Soren Hansen so...@linux2go.dk 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 so...@linux2go.dk --- 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
Re: [libvirt] [PATCH] Allow chardev of type 'file' for UML domains.
On 19-08-2010 15:41, Daniel P. Berrange wrote: + 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'. Not in particular. I was just in a hurry, wanted to share the patch sooner rather than later and then attack the pipes later on. I'll fix it up, but perhaps I won't get to it today. 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. True. I somehow thought virExecWhatever took care of that, but I see now that it doesn't, and I guess that would be troublesome. I'll handle this in the uml driver. -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Allow chardev of type 'file' for UML domains.
From: Soren Hansen so...@linux2go.dk 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. Signed-off-by: Soren Hansen so...@linux2go.dk --- src/uml/uml_conf.c | 30 +- src/uml/uml_conf.h |1 + src/uml/uml_driver.c |6 +- 3 files changed, 27 insertions(+), 10 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 */ 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..5f73ce2 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -737,10 +737,6 @@ static int umlMonitorCommand(const struct uml_driver *driver, virReportSystemError(errno, _(cannot read reply %s), cmd); goto error; } -if (nbytes sizeof res) { -virReportSystemError(0, _(incomplete reply %s), cmd); -goto error; -} if (sizeof res.data res.length) { virReportSystemError(0, _(invalid length in reply %s), cmd); goto error; @@ -871,7 +867,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); -- 1.7.0.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Allow chardev of type 'file' for UML domains.
On 18-08-2010 12:45, so...@linux2go.dk wrote: diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 04493ba..5f73ce2 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -737,10 +737,6 @@ static int umlMonitorCommand(const struct uml_driver *driver, virReportSystemError(errno, _(cannot read reply %s), cmd); goto error; } -if (nbytes sizeof res) { -virReportSystemError(0, _(incomplete reply %s), cmd); -goto error; -} if (sizeof res.data res.length) { virReportSystemError(0, _(invalid length in reply %s), cmd); goto error; Whoops, this bit wasn't meant to be included. /me reposts without it. -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Allow chardev of type 'file' for UML domains.
From: Soren Hansen so...@linux2go.dk 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. Signed-off-by: Soren Hansen so...@linux2go.dk --- 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 */ 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); -- 1.7.0.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list