[libvirt] [PATCH] Fix PolicyKit.conf example docs
The current docs showing a possible PolicyKit.conf with the example: With PolicyKit-0.6-2.fc8, polkit-config-file-validate complains about this format. 'man PolicyKit.conf' gives a similar example with the format: This doesn't cause any complaints and works. The attached patch updates the docs with this format. Thanks, Cole diff --git a/docs/auth.html.in b/docs/auth.html.in index 7340360..04b1210 100644 --- a/docs/auth.html.in +++ b/docs/auth.html.in @@ -77,11 +77,15 @@ while requiring joe to authenticate with the admin password, would require adding the following snippet to PolicyKit.conf. -- + + -+ + - + + Username/password auth -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list+ +
[libvirt] [PATCH] Implement autostart commands for xen
The attached patch implements the domain autostart commands for xen. The xen sexpr (since at least 3.0.4 = 1.5 years) has a on_xend_start field which can be used to autostart a domain. A couple things: 1) This works on a running guest, but will only show the sexpr changes after the guest is restarted. Just curious if there is any better way to do this? 2) This isn't implemented for the xm config file driver. I figured this is fine since this is inherently dependent on inactive domain management. Any feedback is appreciated. Thanks, Cole diff --git a/src/xen_unified.c b/src/xen_unified.c index 91502dc..cf0a68d 100644 --- a/src/xen_unified.c +++ b/src/xen_unified.c @@ -1128,6 +1128,34 @@ xenUnifiedDomainDetachDevice (virDomainPtr dom, const char *xml) return -1; } +static int +xenUnifiedDomainGetAutostart (virDomainPtr dom, int *autostart) +{ +GET_PRIVATE(dom->conn); +int i; + +for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) +if (priv->opened[i] && drivers[i]->domainGetAutostart && +drivers[i]->domainGetAutostart (dom, autostart) == 0) +return 0; + +return -1; +} + +static int +xenUnifiedDomainSetAutostart (virDomainPtr dom, int autostart) +{ +GET_PRIVATE(dom->conn); +int i; + +for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) +if (priv->opened[i] && drivers[i]->domainSetAutostart && +drivers[i]->domainSetAutostart (dom, autostart) == 0) +return 0; + +return -1; +} + static char * xenUnifiedDomainGetSchedulerType (virDomainPtr dom, int *nparams) { @@ -1291,6 +1319,8 @@ static virDriver xenUnifiedDriver = { .domainUndefine= xenUnifiedDomainUndefine, .domainAttachDevice= xenUnifiedDomainAttachDevice, .domainDetachDevice= xenUnifiedDomainDetachDevice, +.domainGetAutostart = xenUnifiedDomainGetAutostart, +.domainSetAutostart = xenUnifiedDomainSetAutostart, .domainGetSchedulerType= xenUnifiedDomainGetSchedulerType, .domainGetSchedulerParameters = xenUnifiedDomainGetSchedulerParameters, .domainSetSchedulerParameters = xenUnifiedDomainSetSchedulerParameters, diff --git a/src/xend_internal.c b/src/xend_internal.c index f9db571..226ba8e 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -115,8 +115,8 @@ struct xenUnifiedDriver xenDaemonDriver = { xenDaemonDomainUndefine, /* domainUndefine */ xenDaemonAttachDevice, /* domainAttachDevice */ xenDaemonDetachDevice, /* domainDetachDevice */ -NULL, /* domainGetAutostart */ -NULL, /* domainSetAutostart */ +xenDaemonDomainGetAutostart, /* domainGetAutostart */ +xenDaemonDomainSetAutostart, /* domainSetAutostart */ xenDaemonGetSchedulerType, /* domainGetSchedulerType */ xenDaemonGetSchedulerParameters, /* domainGetSchedulerParameters */ xenDaemonSetSchedulerParameters, /* domainSetSchedulerParameters */ @@ -3718,6 +3718,99 @@ xenDaemonDetachDevice(virDomainPtr domain, const char *xml) "type", class, "dev", ref, "force", "0", "rm_cfg", "1", NULL)); } +int +xenDaemonDomainGetAutostart(virDomainPtr domain, +int *autostart) +{ +struct sexpr *root; +const char *tmp; + +if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { +virXendError((domain ? domain->conn : NULL), VIR_ERR_INVALID_ARG, + __FUNCTION__); +return (-1); +} + +root = sexpr_get(domain->conn, "/xend/domain/%s?detail=1", domain->name); +if (root == NULL) { +virXendError (domain->conn, VIR_ERR_XEN_CALL, + _("xenDaemonGetAutostart failed to find this domain")); +return (-1); +} + +*autostart = 0; + +tmp = sexpr_node(root, "domain/on_xend_start"); +if (tmp && STREQ(tmp, "start")) { +*autostart = 1; +} + +sexpr_free(root); +return 0; +} + +int +xenDaemonDomainSetAutostart(virDomainPtr domain, +int autostart) +{ +struct sexpr *root, *autonode; +const char *autostr; +char buf[4096]; +int ret = -1; + +if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { +virXendError((domain ? domain->conn : NULL), VIR_ERR_INTERNAL_ERROR, + __FUNCTION__); +return (-1); +} + +root = sexpr_get(domain->conn, "/xend/domain/%s?detail=1", domain->name); +if (root == NULL) { +virXendError (domain->conn, VIR_ERR_XEN_CALL, + _("xenDaemonSetAutostart failed to find this domain")); +return (-1); +} + +autostr = sexpr_node(root, "domain/on_xend_start"); +if (autostr) { +if (!STREQ(autostr, "ignore") && !STREQ(autostr, "start")) { +virXendError(domain->conn, VIR_ERR_INTERNAL_ERROR, + _("unexpected value from on_xend_start")); +goto error; +} + +
Re: [libvirt] Re: [Libvir] [PATCH][RFC] libvirt ldoms support
On Wed, May 07, 2008 at 03:14:52PM -0500, Eunice Moon wrote: > Hi Daniel, > > With the new LDoms release (LDoms 1.1), we plan to provide a more > generic XML format which will be using the OVF (Open Virtual Machine > Format) spec: > http://www.vmware.com/pdf/ovf_spec_draft.pdf > > Even though the OVF spec is still in the Draft stage, it can be > the hypervisor independent XML format for virtual machines. > I am wondering if it is in your radar or have any plan to move > forward to using more generic XML format. OVF is a way to describe the generic functional requirements of a virtual machine (primarily, but not exclusively, for purposes of distribution of appliances). The libvirt XML format is describing a specific deployment of a virtual machine on a specific hypervisor. This is a subtle by very important distinction. For example, OVF may specify that a VM requires 2 disks, of size X and Y. It does not care about the path the disks are stored at, what container format they use etc. The libvirt XML format for a particular deployment of a vm does have to specify the paths to the disks, their format (qcow, vmdk) and many other aspects. The OVF format is akin to the 'virt-image' XML format for distributing virtual appliances, except that OVF is vastly more complex. So in summary there is no intention to use OVF in libvirt because it is serving a different purpose. Dan. -- |: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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
[libvirt] Re: [Libvir] [PATCH] lxc: handle SIGCHLD from exiting container
Jim Meyering wrote: > Dave Leskovec <[EMAIL PROTECTED]> wrote: >> Jim Meyering wrote: >>> Dave Leskovec <[EMAIL PROTECTED]> wrote: ... >> Index: b/qemud/qemud.c >> === > ... >> +static void sig_handler(int sig, siginfo_t * siginfo, >> +void* context ATTRIBUTE_UNUSED) { > ... >> -unsigned char sigc; >> +siginfo_t siginfo; >> int ret; >> >> -if (read(server->sigread, &sigc, 1) != 1) { >> +if (read(server->sigread, &siginfo, sizeof(siginfo)) != >> sizeof(siginfo)) { > > Looks good, but that should be saferead instead of "read". > Now that it's reading more than one byte, EINTR can make a difference. > This latest version of this patch changes the read() to a saferead() as Jim points out. -- Best Regards, Dave Leskovec IBM Linux Technology Center Open Virtualization --- qemud/qemud.c | 26 ++ src/driver.h|4 + src/internal.h |2 src/libvirt.c | 11 src/libvirt_sym.version |1 src/lxc_driver.c| 114 +++- src/qemu_driver.c |1 src/remote_internal.c |1 src/storage_driver.c|1 9 files changed, 120 insertions(+), 41 deletions(-) Index: b/qemud/qemud.c === --- a/qemud/qemud.c 2008-04-28 02:09:52.0 -0700 +++ b/qemud/qemud.c 2008-05-07 13:03:31.0 -0700 @@ -109,16 +109,16 @@ static sig_atomic_t sig_errors = 0; static int sig_lasterrno = 0; -static void sig_handler(int sig) { -unsigned char sigc = sig; +static void sig_handler(int sig, siginfo_t * siginfo, +void* context ATTRIBUTE_UNUSED) { int origerrno; int r; -if (sig == SIGCHLD) /* We explicitly waitpid the child later */ -return; +/* set the sig num in the struct */ +siginfo->si_signo = sig; origerrno = errno; -r = safewrite(sigwrite, &sigc, 1); +r = safewrite(sigwrite, siginfo, sizeof(*siginfo)); if (r == -1) { sig_errors++; sig_lasterrno = errno; @@ -232,10 +232,10 @@ int events ATTRIBUTE_UNUSED, void *opaque) { struct qemud_server *server = (struct qemud_server *)opaque; -unsigned char sigc; +siginfo_t siginfo; int ret; -if (read(server->sigread, &sigc, 1) != 1) { +if (saferead(server->sigread, &siginfo, sizeof(siginfo)) != sizeof(siginfo)) { qemudLog(QEMUD_ERR, _("Failed to read from signal pipe: %s"), strerror(errno)); return; @@ -243,7 +243,7 @@ ret = 0; -switch (sigc) { +switch (siginfo.si_signo) { case SIGHUP: qemudLog(QEMUD_INFO, "%s", _("Reloading configuration on SIGHUP")); if (virStateReload() < 0) @@ -253,11 +253,15 @@ case SIGINT: case SIGQUIT: case SIGTERM: -qemudLog(QEMUD_WARN, _("Shutting down on signal %d"), sigc); +qemudLog(QEMUD_WARN, _("Shutting down on signal %d"), + siginfo.si_signo); server->shutdown = 1; break; default: +qemudLog(QEMUD_INFO, _("Received signal %d, dispatching to drivers"), + siginfo.si_signo); +virStateSigDispatcher(&siginfo); break; } @@ -2186,8 +2190,8 @@ goto error1; } -sig_action.sa_handler = sig_handler; -sig_action.sa_flags = 0; +sig_action.sa_sigaction = sig_handler; +sig_action.sa_flags = SA_SIGINFO; sigemptyset(&sig_action.sa_mask); sigaction(SIGHUP, &sig_action, NULL); Index: b/src/driver.h === --- a/src/driver.h 2008-04-10 09:54:54.0 -0700 +++ b/src/driver.h 2008-05-05 23:28:40.0 -0700 @@ -11,6 +11,8 @@ #include +#include + #ifdef __cplusplus extern "C" { #endif @@ -565,6 +567,7 @@ typedef int (*virDrvStateCleanup) (void); typedef int (*virDrvStateReload) (void); typedef int (*virDrvStateActive) (void); +typedef int (*virDrvSigHandler) (siginfo_t *siginfo); typedef struct _virStateDriver virStateDriver; typedef virStateDriver *virStateDriverPtr; @@ -574,6 +577,7 @@ virDrvStateCleanup cleanup; virDrvStateReload reload; virDrvStateActive active; +virDrvSigHandler sigHandler; }; /* Index: b/src/internal.h === --- a/src/internal.h 2008-04-28 14:44:54.0 -0700 +++ b/src/internal.h 2008-04-30 15:01:24.0 -0700 @@ -344,10 +344,12 @@ int __virStateCleanup(void); int __virStateReload(void); int __virStateActive(void); +int __virStateSigDispatcher(siginfo_t *siginfo); #define virStateInitialize() __virStateInitialize() #define virStateCleanup() __virStateCleanup() #define virStateReload() __virStateReload() #define
[libvirt] Re: [Libvir] [PATCH][RFC] libvirt ldoms support
Hi Daniel, With the new LDoms release (LDoms 1.1), we plan to provide a more generic XML format which will be using the OVF (Open Virtual Machine Format) spec: http://www.vmware.com/pdf/ovf_spec_draft.pdf Even though the OVF spec is still in the Draft stage, it can be the hypervisor independent XML format for virtual machines. I am wondering if it is in your radar or have any plan to move forward to using more generic XML format. For LDoms Manager, we will support the backward compatibility with the current LDoms XML format since other tools/applications are still using the current XML format. But, for the ldoms libvirt patch, would it be the acceptable format if we use the OVF-style XML format? Thanks, Eunice Daniel Veillard wrote: On Thu, Apr 10, 2008 at 04:10:02PM -0500, Eunice Moon wrote: Hi, As John mentioned, the LDoms Manager (ldm) XML interface has been used by a different set of software such as SNMP MIB, BUI, and other tools/applications (before the libvirt), so we can't really change the ldm XML formats. I understood the existing format comes from other legacy uses, no problem for me, looking at the second XML example made that clear :-) But, I will look into the way (conversion between the LDoms XML and libvirt XML formats) to make it compatible with the official libvirt. I can give an hand, it's probably also a good opportunity to refactor a lot of the XML handling code coming from the patch since we will have to change the way things are done. The hardest for me is understand the semantic of the current document, we need this anyway to find how to best map things onto the libvirt XML format (and back). Then for the revamp of the conversion code i can probably do part of it, and probably faster than you considering expertise on libxml2 ;-) But I really need a good description of the current XML format, Daniel -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [Libvir] [PATCH] lxc: loop in tty forwarding process
Daniel Veillard wrote: > On Tue, May 06, 2008 at 12:51:08AM -0700, Dave Leskovec wrote: [...] >> -close(vm->parentTty); >> +//close(vm->parentTty); >> close(vm->containerTtyFd); > > if we really don't need this anymore just remove it, if you have doubts then > maybe this should be clarified. In any case let's stick to old style comments > /* ... */ > That shouldn't be commented out. I've restored it in the attached updated patch. -- Best Regards, Dave Leskovec IBM Linux Technology Center Open Virtualization --- src/lxc_driver.c | 172 --- 1 file changed, 128 insertions(+), 44 deletions(-) Index: b/src/lxc_driver.c === --- a/src/lxc_driver.c 2008-05-05 23:21:56.0 -0700 +++ b/src/lxc_driver.c 2008-05-07 11:04:38.0 -0700 @@ -26,9 +26,10 @@ #ifdef WITH_LXC #include -#include +#include #include #include +#include #include #include #include @@ -552,7 +553,7 @@ int rc = -1; char tempTtyName[PATH_MAX]; -*ttymaster = posix_openpt(O_RDWR|O_NOCTTY); +*ttymaster = posix_openpt(O_RDWR|O_NOCTTY|O_NONBLOCK); if (*ttymaster < 0) { lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR, _("posix_openpt failed: %s"), strerror(errno)); @@ -593,75 +594,155 @@ } /** + * lxcFdForward: + * @readFd: file descriptor to read + * @writeFd: file desriptor to write + * + * Reads 1 byte of data from readFd and writes to writeFd. + * + * Returns 0 on success, EAGAIN if returned on read, or -1 in case of error + */ +static int lxcFdForward(int readFd, int writeFd) +{ +int rc = -1; +char buf[2]; + +if (1 != (saferead(readFd, buf, 1))) { +if (EAGAIN == errno) { +rc = EAGAIN; +goto cleanup; +} + +lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("read of fd %d failed: %s"), readFd, strerror(errno)); +goto cleanup; +} + +if (1 != (safewrite(writeFd, buf, 1))) { +lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("write to fd %d failed: %s"), writeFd, strerror(errno)); +goto cleanup; +} + +rc = 0; + +cleanup: +return rc; +} + +typedef struct _lxcTtyForwardFd_t { +int fd; +bool active; +} lxcTtyForwardFd_t; + +/** * lxcTtyForward: * @fd1: Open fd * @fd1: Open fd * * Forwards traffic between fds. Data read from fd1 will be written to fd2 - * Data read from fd2 will be written to fd1. This process loops forever. + * This process loops forever. + * This uses epoll in edge triggered mode to avoid a hard loop on POLLHUP + * events when the user disconnects the virsh console via ctrl-] * * Returns 0 on success or -1 in case of error */ static int lxcTtyForward(int fd1, int fd2) { int rc = -1; -int i; -char buf[2]; -struct pollfd fds[2]; -int numFds = 0; - -if (0 <= fd1) { -fds[numFds].fd = fd1; -fds[numFds].events = POLLIN; -++numFds; +int epollFd; +struct epoll_event epollEvent; +int numEvents; +int numActive = 0; +lxcTtyForwardFd_t fdArray[2]; +int timeout = -1; +int curFdOff = 0; +int writeFdOff = 0; + +fdArray[0].fd = fd1; +fdArray[0].active = false; +fdArray[1].fd = fd2; +fdArray[1].active = false; + +/* create the epoll fild descriptor */ +epollFd = epoll_create(2); +if (0 > epollFd) { +lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("epoll_create(2) failed: %s"), strerror(errno)); +goto cleanup; } -if (0 <= fd2) { -fds[numFds].fd = fd2; -fds[numFds].events = POLLIN; -++numFds; +/* add the file descriptors the epoll fd */ +memset(&epollEvent, 0x00, sizeof(epollEvent)); +epollEvent.events = EPOLLIN|EPOLLET;/* edge triggered */ +epollEvent.data.fd = fd1; +epollEvent.data.u32 = 0;/* fdArray position */ +if (0 > epoll_ctl(epollFd, EPOLL_CTL_ADD, fd1, &epollEvent)) { +lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("epoll_ctl(fd1) failed: %s"), strerror(errno)); +goto cleanup; } - -if (0 == numFds) { -DEBUG0("No fds to monitor, return"); +epollEvent.data.fd = fd2; +epollEvent.data.u32 = 1;/* fdArray position */ +if (0 > epoll_ctl(epollFd, EPOLL_CTL_ADD, fd2, &epollEvent)) { +lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("epoll_ctl(fd2) failed: %s"), strerror(errno)); goto cleanup; } while (1) { -if ((rc = poll(fds, numFds, -1)) <= 0) { +/* if active fd's, return if no events, else wait forever */ +timeout = (numActive > 0) ? 0 : -1; +numEvents = epoll_wait(epollFd, &epollEvent, 1, timeout); +if (0 < numEvents) { +if (epollEvent.events & EPOLLIN) { +curFdOff = epollEve
[libvirt] Re: [Libvir] [PATCH] Implement SetVcpus and DomainGetMaxVcpus for qemu
Daniel P. Berrange wrote: >> >> /* XXX future KVM will support SMP. Need to probe >> kernel to figure out KVM module version i guess */ >> -if (!strcmp(type, "kvm")) >> +if (!strcasecmp(type, "kvm")) >> return 1; > > This comment is seriously out of date - KVM supports 16 (or was is 32?) vCPUs, > so we should change this. > According to the kvm changelog, 16 vcpus is supported since kvm-62. Prior to that (such as on f8) only 4 seem to be supported. - Cole -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [Libvir] [PATCH] Implement SetVcpus and DomainGetMaxVcpus for qemu
On Tue, May 06, 2008 at 12:35:12PM -0400, Cole Robinson wrote: > Cole Robinson wrote: > > The attached patch fills in two of the vcpu functions for the qemu driver: > > > > virDomainSetVcpus : set the number of vcpus the domain can use > > virDomainGetMaxVcpus : max number of vcpus that can be assigned to the > > domain. > > > > Updated patch. Fixes the issues pointed out by Jim and Rich. I also realized > I was passing the incorrect value to qemudMaxVCPUS, so to help fix this I > added > a function to qemu_conf.c to translate a domain's virtType value to the > corresponding string. Okidoc, looks fine, applied and commited, thanks ! Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Re: [Libvir] [PATCH] sound support for qemu and xen
On Tue, May 06, 2008 at 10:25:37PM +0100, Daniel P. Berrange wrote: > On Tue, May 06, 2008 at 10:39:53AM -0400, Cole Robinson wrote: > > Cole Robinson wrote: > > > The patch below adds xml support for the soundhw option to qemu > > > and xen. > > > > Third iteration. Changes include: > > > > - Rediffd against current code > > - Reading 'all' from a xend sexpr or xm config does the right thing. > > - Added test files. > > Great, this looks good now Excellent, this is now commited. One thing left to do is document the associated XML extentions for the devices sections. Should be easier now with Dan's new HTML generation, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Re: [Libvir] [PATCH] lxc: loop in tty forwarding process
On Wed, May 07, 2008 at 03:45:46PM +0200, Jim Meyering wrote: > Daniel Veillard <[EMAIL PROTECTED]> wrote: > > On Tue, May 06, 2008 at 12:51:08AM -0700, Dave Leskovec wrote: > > Sounds fine in principle but i have a couple of questions with the patch > > > >> +#include > > > > err ... what is that ? looks like a linux specific header, do we really > > need this ? epoll is linux specific I think but #include > > should be sufficient no ? > > is not Linux specific. > It's the C99-specified header that provides e.g,. the "bool" type. > > Good C code has been able to use the "bool" type portably (at least > through autoconf/gnulib-provided insulation) for many years. > These days you rarely need the compatibility shims, > since nearly everyone has a c99-compliant compiler. Okay, first time I see it, didn't found it in the standard path, and the man page about it was looking suspicious to me :-) Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Re: [Libvir] [PATCH] lxc: loop in tty forwarding process
Daniel Veillard <[EMAIL PROTECTED]> wrote: > On Tue, May 06, 2008 at 12:51:08AM -0700, Dave Leskovec wrote: > Sounds fine in principle but i have a couple of questions with the patch > >> +#include > > err ... what is that ? looks like a linux specific header, do we really > need this ? epoll is linux specific I think but #include > should be sufficient no ? is not Linux specific. It's the C99-specified header that provides e.g,. the "bool" type. Good C code has been able to use the "bool" type portably (at least through autoconf/gnulib-provided insulation) for many years. These days you rarely need the compatibility shims, since nearly everyone has a c99-compliant compiler. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Re: [Libvir] [PATCH] lxc: loop in tty forwarding process
On Wed, May 07, 2008 at 08:25:58AM -0400, Daniel Veillard wrote: > On Tue, May 06, 2008 at 12:51:08AM -0700, Dave Leskovec wrote: > > This patch changes the lxc tty forwarding process to use epoll instead of > > poll. > > This is done to avoid a cpu consuming loop when a user disconnects from the > > container console. > > > > During some testing, we found that when the slave end of a tty is closed, > > calls > > to poll() on the master end will return immediately with POLLHUP until the > > slave > > end is opened again. The result of this is that if you connect to the > > container > > console using virsh and then ctrl-] out of it, the container tty process > > will > > chew up the processor handling POLLHUP. This event can't be disabled > > regardless > > of what is set in the events field. > > > > This can be avoided by using epoll. When used in edge triggered mode, you > > see > > the initial EPOLLHUP but will not see another one until someone connects and > > then disconnects from the console again. This also drove some changes into > > how > > the regular input data is handled. Once an EPOLLIN is seen from an fd, > > another > > one will not be surfaced until all data has been read from the file (EAGAIN > > is > > returned by read). > > Sounds fine in principle but i have a couple of questions with the patch > > > +#include > > err ... what is that ? looks like a linux specific header, do we really > need this ? epoll is linux specific I think but #include > should be sufficient no ? It is kinda academic whether epool is linux specific - the entire LXC driver is Linux specific, so you're not compiling it on Solaris/Windows anyway. So any Linux-isms in LXC driver code is fine Dan. -- |: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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: Add bus attribute for Xen driver
On Wed, May 07, 2008 at 01:17:46AM +0100, Daniel P. Berrange wrote: > To complement soren's patch adding a bus attribute to the QEMU driver, > here is a minimal patch adding bus attribute to the Xen drivers. It merely > adds it on when generating the XML. It isn't making any attempt to interpret > it when creating a VM, since Xen does everything based off the disk node > name anyway its (currently) redundant. > > The bus types supported are 'xen' for paravirt disks, or 'ide' and 'scsi' > for HVM guests. +1, this helps migrating from one hypervisor to another (or detecting if the migration would be harder), Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [Libvir] [PATCH] lxc: loop in tty forwarding process
On Tue, May 06, 2008 at 12:51:08AM -0700, Dave Leskovec wrote: > This patch changes the lxc tty forwarding process to use epoll instead of > poll. > This is done to avoid a cpu consuming loop when a user disconnects from the > container console. > > During some testing, we found that when the slave end of a tty is closed, > calls > to poll() on the master end will return immediately with POLLHUP until the > slave > end is opened again. The result of this is that if you connect to the > container > console using virsh and then ctrl-] out of it, the container tty process will > chew up the processor handling POLLHUP. This event can't be disabled > regardless > of what is set in the events field. > > This can be avoided by using epoll. When used in edge triggered mode, you see > the initial EPOLLHUP but will not see another one until someone connects and > then disconnects from the console again. This also drove some changes into > how > the regular input data is handled. Once an EPOLLIN is seen from an fd, > another > one will not be surfaced until all data has been read from the file (EAGAIN is > returned by read). Sounds fine in principle but i have a couple of questions with the patch > +#include err ... what is that ? looks like a linux specific header, do we really need this ? epoll is linux specific I think but #include should be sufficient no ? [...] > > -close(vm->parentTty); > +//close(vm->parentTty); > close(vm->containerTtyFd); if we really don't need this anymore just remove it, if you have doubts then maybe this should be clarified. In any case let's stick to old style comments /* ... */ Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Re: [Libvir] [PATCH] Add bus attribute to disk target definition
On Wed, May 07, 2008 at 09:38:26AM +0200, Soren Hansen wrote: > Yes and no. In the latest patch I provided I only set use_extboot if > there's only one boot device defined, and it's a virtio device, so PXE > booting would use the old-style "-boot n" syntax. I literallly woke up > this morning and instantly smacked my forehead due to another problem > this introduced, so I'm happy you changed it. :) The thing is that you can specify multiple boot devices in the XML to set a priority, eg Gets maps to '--boot nc' so in this scenario you'd have a virtio device being bootable, and also have PXE enabled. > Oh, and thanks for doing all the test cases as well. I didn't want to > get started on those until we had agreed on the logic that should be > applied. FWIW, I always start with the test cases first defining the XML and QEMU args, and then write the impl until the test cases pass. Dan. -- |: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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] Re: [Libvir] [PATCH] Add bus attribute to disk target definition
On Tue, May 06, 2008 at 10:02:21PM +0100, Daniel P. Berrange wrote: > > sorry for the delay. Here's the newest version of the patch which > > should address all the issues that have been raised so far. > Yes & no. It didn't address the redundant re-ordering of -drive > parameters when using boot=on, The reasoning here was (I mentioned this in a previous mail, too) that when qemu/kvm some day grows the ability to have more than one boot device specified with boot=on (using extboot or whatever), we're going to have to change things *anyway*. Ordering the devices according to boot priority seems like a reasonable guess as to what would be required to do, so I figured I'd leave it as is. > nor re-add the -boot param to make PXE work again. Yes and no. In the latest patch I provided I only set use_extboot if there's only one boot device defined, and it's a virtio device, so PXE booting would use the old-style "-boot n" syntax. I literallly woke up this morning and instantly smacked my forehead due to another problem this introduced, so I'm happy you changed it. :) > One further complication is that QEMU 0.9.1 has -drive support but not > the extboot support, so boot=on can't be used there. It rather > annoyingly complains > > qemu: unknowm parameter 'boot' in > 'file=/home/berrange/boot.iso,media=cdrom,boot=on' Ah, figures. >> +if (!bus) >> +disk->bus = QEMUD_DISK_BUS_IDE; > This was giving floppy disks a bus of 'ide' which isn't technically > correct - they're really their own bus type - I reckon we should call > it 'fdc'. Ah. Yes, I must admit that floppy disks were completely off my radar. > This double loop is redundant - there's no need to pull bootable > drives to the front of the list - this is why there's an explicit flag > rather than relying on ordering. I did this for two reasons: a) I wanted to avoid the bootDisk, bootFloppy, bootCD variables approach you used. It just didn't appeal to me. *shrug* b) As I mentioned further up, this was also done in an attempt to match what would be needed when it becomes possible to specify ",boot=on" for more than one device, but we can revisit this when that day comes. Your patch looks fine to me. Oh, and thanks for doing all the test cases as well. I didn't want to get started on those until we had agreed on the logic that should be applied. -- Soren Hansen | Virtualisation specialist | Ubuntu Server Team Canonical Ltd. | http://www.ubuntu.com/ signature.asc Description: Digital signature -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list