[libvirt] reload libvirtd.conf has a little problem.
hi When libvirtd is running, I changed unix_sock_ro_perms=0777 into unix_sock_ro_perms=0755, and then reload the configuration with: kill -SIGHUP `pidof libvirtd` or service libvirtd reload the permission of /var/lib/libvirt/libvirt-sock-ro still is 0777, but not 0755. and after service libvirtd restart, the permission is changed into 0755. think it's a little problem. libvirt version: libvirt-0.7.6-1.fc12.x86_64 Thanks and Regards osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: avoid warning about unused variables
On Thu, Feb 25, 2010 at 02:28:10PM +0100, Jim Meyering wrote: From 8ddff3e6cdccc2b4289509c6941b43f2ddf8e643 Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Thu, 25 Feb 2010 14:19:33 +0100 Subject: [PATCH] build: avoid warning about unused variables * tools/virsh.c (cmdCPUBaseline): Remove declarations of unused variables, p and cur. --- tools/virsh.c |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 5fdbbe5..89eefcf 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -7048,12 +7048,11 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) int found; int ret = TRUE; char *buffer; -char *p; char *result = NULL; const char **list = NULL; unsigned int count = 0; xmlDocPtr doc = NULL; -xmlNodePtr node_list, cur; +xmlNodePtr node_list; xmlXPathContextPtr ctxt = NULL; xmlSaveCtxtPtr sctxt = NULL; xmlBufferPtr buf = NULL; Heh, thanks, I'm surprized I didn't see it, maybe because I built without optimization when developping the patch... Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] reload libvirtd.conf has a little problem.
On Mon, Mar 01, 2010 at 05:30:21PM +0800, Osier Yang wrote: hi When libvirtd is running, I changed unix_sock_ro_perms=0777 into unix_sock_ro_perms=0755, and then reload the configuration with: kill -SIGHUP `pidof libvirtd` or service libvirtd reload the permission of /var/lib/libvirt/libvirt-sock-ro still is 0777, but not 0755. and after service libvirtd restart, the permission is changed into 0755. think it's a little problem. libvirt version: libvirt-0.7.6-1.fc12.x86_64 libvirtd never attempts to reload /etc/libvirt/libvirtd.conf while running. The SIGHUP only triggers a reload of the hypervisor drivers' guest/network configuration files. 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] Hypervisor raw options
On Fri, Feb 26, 2010 at 07:47:20PM +0100, Daniel Dehennin wrote: Hello, I wonder if it's possible to add hypervisor pass-through options, I add some usage of kvm -startdate and need to use a script which call kvm with the desired option. Something like: emulator/usr/bin/kvm/emulator emulator-options option-option1/option option-option2/option /emulator-options The several option/ will be passed to the emulator/ command. The goal of the libvirt XML format is to provide a representation that is consistent across all hypervisors, so that regardless of whether using Xen, KVM, or VMWare an config parameter can be encoded that same way. If we were to support your suggestion, every hypervisor in libvirt would end up with a different XML configuration. In addition KVM / QEMU have a history of renaming/breaking command line options, so there's no way of guarenteeing that '-foo' from QEMU 0.9.1 still works with QEMU 0.10.0 or vica-verca. If you have specific KVM arguments that libvirt does not yet support then please either file a RFE bug report with details, or raise the issue on this mailing list WRT your example of -startdate, I posted patches for supporting that last week http://www.redhat.com/archives/libvir-list/2010-February/msg00104.html 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 4/9] Add QEMU driver support for job info on migration ops
On Fri, Feb 26, 2010 at 06:51:05PM +0100, Daniel Veillard wrote: On Thu, Feb 18, 2010 at 03:56:10PM +, Daniel P. Berrange wrote: Introduce support for virDomainGetJobInfo in the QEMU driver. This allows for monitoring of any API that uses the 'info migrate' monitor command. ie virDomainMigrate, virDomainSave and virDomainCoreDump Unfortunately QEMU does not provide a way to monitor incoming migration so we can't wire up virDomainRestore yet. The virsh tool gets a new command 'domjobinfo' to query status * src/qemu/qemu_driver.c: Record virDomainJobInfo and start time in qemuDomainObjPrivatePtr objects. Add generic shared handler for calling 'info migrate' with all migration based APIs. * src/qemu/qemu_monitor_text.c: Fix parsing of 'info migration' reply * tools/virsh.c: add new 'domjobinfo' command to query progress --- src/qemu/qemu_driver.c | 208 +++--- src/qemu/qemu_monitor_text.c |7 +- tools/virsh.c| 129 ++ 3 files changed, 288 insertions(+), 56 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a6dc4f9..b245eb2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -87,6 +87,8 @@ struct _qemuDomainObjPrivate { int jobActive; /* Non-zero if a job is active. Only 1 job is allowed at any time * A job includes *all* monitor commands, even those just querying * information, not merely actions */ +virDomainJobInfo jobInfo; +unsigned long long jobStart; qemuMonitorPtr mon; virDomainChrDefPtr monConfig; @@ -329,6 +331,8 @@ static int qemuDomainObjBeginJob(virDomainObjPtr obj) } } priv-jobActive = 1; +priv-jobStart = (now.tv_sec * 1000ull) + (now.tv_usec / 1000); +memset(priv-jobInfo, 0, sizeof(priv-jobInfo)); return 0; } @@ -373,6 +377,8 @@ static int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver, } } priv-jobActive = 1; +priv-jobStart = (now.tv_sec * 1000ull) + (now.tv_usec / 1000); +memset(priv-jobInfo, 0, sizeof(priv-jobInfo)); Hum, I though the time was measured in second in the job strcture, why keep a millisecond timestamp here ? Maybe seconds are too coarse in the API anyway Yeah, the public API comment was wrong - we were passing milliseconds out to the client app, and virsh is processing it as milliseconds. virDomainObjUnlock(obj); qemuDriverLock(driver); @@ -395,6 +401,8 @@ static int ATTRIBUTE_RETURN_CHECK qemuDomainObjEndJob(virDomainObjPtr obj) qemuDomainObjPrivatePtr priv = obj-privateData; priv-jobActive = 0; +priv-jobStart = 0; +memset(priv-jobInfo, 0, sizeof(priv-jobInfo)); virCondSignal(priv-jobCond); return virDomainObjUnref(obj); @@ -3919,6 +3927,96 @@ cleanup: } +static int +qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr vm) +{ +int ret = -1; +int status; +unsigned long long memProcessed; +unsigned long long memRemaining; +unsigned long long memTotal; +qemuDomainObjPrivatePtr priv = vm-privateData; + +priv-jobInfo.type = VIR_DOMAIN_JOB_UNBOUNDED; + +while (priv-jobInfo.type == VIR_DOMAIN_JOB_UNBOUNDED) { +/* Poll every 1/2 second for progress to allow cancellation */ that's way larger than an human perception window so from an UI perspective the user will percieve the time between the request and when it takes effect, probably not a big deal but could be refined in the future. Migration still starts immediately, we merely have a 1/2 second delay for detecting its completion. We need to improve this regardless though since migraiton is supposed to be seemless ! +struct timespec ts = { .tv_sec = 0, .tv_nsec = 500 * 1000ull }; +struct timeval now; +int rc; + +qemuDomainObjEnterMonitorWithDriver(driver, vm); ACK, 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] reload libvirtd.conf has a little problem.
hmmm, I see. Thanks. Regards osier Daniel P. Berrange wrote: On Mon, Mar 01, 2010 at 05:30:21PM +0800, Osier Yang wrote: hi When libvirtd is running, I changed unix_sock_ro_perms=0777 into unix_sock_ro_perms=0755, and then reload the configuration with: kill -SIGHUP `pidof libvirtd` or service libvirtd reload the permission of /var/lib/libvirt/libvirt-sock-ro still is 0777, but not 0755. and after service libvirtd restart, the permission is changed into 0755. think it's a little problem. libvirt version: libvirt-0.7.6-1.fc12.x86_64 libvirtd never attempts to reload /etc/libvirt/libvirtd.conf while running. The SIGHUP only triggers a reload of the hypervisor drivers' guest/network configuration files. Regards, Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Free the macvtap mode string
On Fri, Feb 26, 2010 at 01:03:56PM -0500, Stefan Berger wrote: I forgot to free the macvtap mode string. This fixes it. Signed-off-by: Stefan Berger stef...@us.ibm.com Index: libvirt-macvtap/src/conf/domain_conf.c === --- libvirt-macvtap.orig/src/conf/domain_conf.c +++ libvirt-macvtap/src/conf/domain_conf.c @@ -1974,6 +1974,7 @@ cleanup: VIR_FREE(type); VIR_FREE(internal); VIR_FREE(devaddr); +VIR_FREE(mode); return def; Ah, right, ACK, pushing it, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] and for LXC?
Manao ahoana, Hello, Bonjour, Is there some plan/schedule to make libvirt LXC friendly? Or is it already? Misaotra, Thanks, Merci. -- Architecte Informatique chez Blueline/Gulfsat: Administration Systeme, Recherche Developpement +261 34 29 155 34 / +261 33 11 207 36 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Problems with attach-device/detach-device using libvirt 0.7.6
On Fri, Feb 26, 2010 at 04:24:11PM +0100, Rolf Eike Beer wrote: Hi, first a short summary of what the situation is: I'm running qemu 0.12.2, libvirt 0.7.6 on a E5520 based machine with kernel 2.6.33. I also tried 2.6.32.7 as I was using that before and still had the old kernel around, but kernel version doesn't matter AFAICT. The host is a gentoo AMD64 installation, the client is SuSE 11.0 (both 32 and 64 bit tested). This system is our machine to run automated PCIe device tests on, so the idea is to move PCIe devices from the host to different vms, run some testcases there and then move them away again. This has worked before (I'm not absolutely sure, but I would bet a bit of money that it was libvirt 0.7.5). So, what I'm going to do is simply virsh # attach-device suse11.0-AMD64 dev-pci-AD1-CL.xml Device attached successfully virsh # detach-device suse11.0-AMD64 dev-pci-AD1-CL.xml Device detached successfully This is just was I expect and what I now get with the two patches attached. The original situation was somewhat different (i.e. broken): -first I got complains in the log stating: tried to create id (null) twice for device Well, adding two devices with the same id is bad, and if that id is a NULL pointer anyway IMHO it's better to not pass the parameter at all. That's what the null-pci-id patch does. The goal of libvirt when using QEMU = 0.12 is that 'id=' parameter is set for all devices. This enables libvirt reliably identify devices when talking to QEMU. Since you were seeing '(null)' there must be a place in the code where we've forgotton to generate the device alias name for hotplug. We should fix that, rather than hiding the problem. -next, detaching failed. I found out that the command sent to qemu was pci_del pci_addr=fc67add0:7f41:fe38b65a which is obviously bogus. After some hours of digging through the command stack up and down I found that it was the memcpy addressed in the pci_memcpy patch. When the device is attached qemu just replies with an empty string. Afterwards from the guestAddr the address of the PCI device in the client is copied back to the info structure. Too bad that this structure was never initialized to anything useful (at least not for me). When I kick out the memcpy() everything works fine. Ok, I see what's wrong here. The code has two ways of attaching devices if (qemuCmdFlags QEMUD_CMD_FLAG_DEVICE) ret = qemuMonitorAddDevice(priv-mon, devstr); else ret = qemuMonitorAddPCIHostDevice(priv-mon, hostdev-source.subsys.u.pci, guestAddr); With the qemuMonitorAddDevice() method, libvirt decides the PCi address ahead of time. In the qemuMonitorAddPCIHostDevice() method, QEMU decides the PCI addresses tells us about it. Thus we only want todo the memcpy() bit in the 'else' case, never the 'if' case. Note: when the client PCI id is passed to qemu anyway it would be cool if that could be specified in the device XML. For USB devices, too. You already can supply the PCI address in the XML - if you run 'virsh dumpxml' on your running guest you'll see the syntax for PCI addresses. For USB devices, the device IDs on the bus are assigned by the guest OS when it activates the USB devices, so I don't think there's any way to allow user specification of that 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] libvirt/dnsmasq integration.
On Wed, Feb 24, 2010 at 02:34:53PM +, Simon Kelley wrote: As the principal maintainer of dnsmasq, I'm seeing increasing reports of problems on systems which run both dnsmasq and libvirt. I'm fairly sure I understand what's going on in these cases, and I have a few proposals for changes in libvir and dnsmasq that should fix things. Thanks for starting this topic - it would certainly be nice if we can come up with a solution that has better inter-operability fewer surprises for administrators. The problem is that libvirt runs a private instance of dnsmasq: on machines which are also running a system dnsmasq daemon, this can cause problems. Some background: dnsmasq can run in two modes. Default mode: dnsmasq binds the wildcard address and does network magic to determine which interface request packets actually come from, so that the results can be sent back with the correct source address. This has the advantage that network interfaces can come and go and change IP address and dnsmasq will keep working. It's possible to restrict dnsmasq to only reply to requests on some interfaces; requests from other interfaces will be read by dnsmasq and then silently dropped. Telling dnsmasq to use an interface which doesn't exist but might in the future will result in a logged warning, but dnsmasq will still start and when the interface comes up it will work. Bind-interfaces mode: This is the traditional way to do UDP servers. At startup dnsmasq enumerates all the extant interfaces and then opens a socket for each one, listening on the interfaces's IP address. Interfaces may be skipped if excluded by the --interface and --except-interface flags, and any interface specified in --interface which doesn't exist at start-up will generate a fatal error. Yep, I remember we hit that fatal error in libvirt, when we create our bridge device then launched dnsmasq, sometimes dnsmasq would exit with an error because the bridge device wasn't visible in userspace yet. Thus we use bind-interfaces mode, but instead of using the flag --interface=virbr0, we switched to --listen-address=IP-of-VIRBR0 I imagine you've already seen this, but as an example of the ARGV that libvirt generates for its dnsmasq instances: /usr/sbin/dnsmasq \ --strict-order \ --bind-interfaces \ --pid-file=/var/run/libvirt/network/default.pid \ --conf-file= \ --listen-address 192.168.122.1 \ --except-interface lo \ --dhcp-range 192.168.122.2,192.168.122.254 \ --dhcp-lease-max=253 In almost all cases, default mode is better: --bind-interfaces is only there to cope with old platforms which don't support enough socket options to do default mode. The only time when --bind-interfaces works better is when it's desirable to run more than one instance of dnsmasq or have dnsmasq co-exist with another DNS server. This is not possible in default mode, but it does work in bind-interfaces mode, providing than _all_ instances of dnsmasq are in bind-interfaces mode, and that they listen on a disjoint set of interfaces. Yes, for want of any alternative, we currently recommend users with a system instance of dnsmasq to use bind-interfaces, and either --interface or --listen-address http://wiki.libvirt.org/page/Libvirtd_and_dnsmasq Therefore, to allow multiple dnsmasq instances libvirt's private dnsmasq instance is started in bind-interfaces mode: that forces one of the dnsmasq instances to do bind-interfaces. Many of the Linux distibution dnsmasq packages have now implemented an /etc/dnsmasq.d directory where configuration fragments can be dropped. Their libvirt packages are putting a file there which contains a bind-interfaces command, so that the system dnsmasq is automatically forced into the same mode, and the two can co-exist. This works, sort-of, but there some disadvantages. Installing libvirt drops the configuration change for the system dnsmasq, but the packages frequently don't restart the system daemon, so that things transiently fail until everything has rebooted. Much worse, the system dnsmasq is forced into bind-interfaces mode and then service to transient interfaces (usb, ad-hoc wifi) no longer works, or, because those interfaces are mentioned in the dnsmasq configuration, dnsmasq now fails at start-up when the interfaces don't exist. Yes, this is rather a pain. Aside from the scheme you propose later, there is one other (hacky) way to deal with this - use a udev script to trigger update + reload of the system dnsmasq's configuration when a USB NIC device hotplug/unplug occurs. That is clearly just crude patch over the already serious problem. My proposal is to get rid of the necessity for two dnsmasq instances. Libvirt should check for the existance of a system dnsmasq and, if the system daemon exists, libvirt should drop the required configuration into /etc/dnsmasq.d and then restart it. If the
Re: [libvirt] and for LXC?
On Mon, Mar 01, 2010 at 04:14:34PM +0300, Mihamina Rakotomandimby wrote: Manao ahoana, Hello, Bonjour, Is there some plan/schedule to make libvirt LXC friendly? Or is it already? http://libvirt.org/ livirt supports: The LXC Linux container system http://libvirt.org/drvlxc.html Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] inability to open local read-only connection
On Thu, Feb 25, 2010 at 02:34:02PM -0600, Tavares, John wrote: I have been experimenting with using libvirt (0.3.3) on a variety of systems (RHEL, CentOS and Oracle VM). I have run into an issue when I try to open a local read-only connection to the hypervisor that is failing only on Oracle VM server release 2.2.0. I have created a root owned setuid executable that is effectively running as root, but even so, still cannot open the local read-only connection of the hypervisor. It only works if I run it directly as root. This is not an option. I do not understand why it works as is on my RHEL and CentOS machines, but not my Oracle machine. It would seem as thought it is not checking if the effective uid is root, just the uid. A readonly connection essentially just goes to a separate UNIX socket, which has more relaxed permissions (mode 0777, instead of 0700). The kernel does the permissions checking when attempting to open it, so it should be using the effective ID. Has anyone run into a similar issue or have any suggestions of what I might try to fix this issue or can tell me that this is a defect that needs (is) fixed?? I'd suggest trying to strace your process see where it fails, and/or run with LIBVIRT_DEBUG=1 environemnt variable set. 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] Free the macvtap mode string
On Fri, Feb 26, 2010 at 01:03:56PM -0500, Stefan Berger wrote: I forgot to free the macvtap mode string. This fixes it. Signed-off-by: Stefan Berger stef...@us.ibm.com Index: libvirt-macvtap/src/conf/domain_conf.c === --- libvirt-macvtap.orig/src/conf/domain_conf.c +++ libvirt-macvtap/src/conf/domain_conf.c @@ -1974,6 +1974,7 @@ cleanup: VIR_FREE(type); VIR_FREE(internal); VIR_FREE(devaddr); +VIR_FREE(mode); return def; ACK 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] qemu: avoid null dereference on failed migration
On Fri, Feb 26, 2010 at 10:17:53AM -0700, Eric Blake wrote: From: Eric Blake ebl...@redhat.com * src/qemu/qemu_monitor_text.c (qemuMonitorTextGetMigrationStatus): Check for failed strchr, to silence a coverity warning. Signed-off-by: Eric Blake ebl...@redhat.com --- src/qemu/qemu_monitor_text.c |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 62ffcc6..e7b4b1f 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -989,6 +989,11 @@ int qemuMonitorTextGetMigrationStatus(qemuMonitorPtr mon, if ((tmp = strstr(reply, MIGRATION_PREFIX)) != NULL) { tmp += strlen(MIGRATION_PREFIX); end = strchr(tmp, '\r'); +if (end == NULL) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, +_(unexpected migration status in %s), reply); +goto cleanup; +} *end = '\0'; if ((*status = qemuMonitorMigrationStatusTypeFromString(tmp)) 0) { -- ACK 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 1/9] Introduce public API for domain async job handling
On Fri, Feb 26, 2010 at 06:40:10PM +0100, Daniel Veillard wrote: On Thu, Feb 18, 2010 at 03:56:07PM +, Daniel P. Berrange wrote: Introduce a new public API that provides a way to get progress info on currently running jobs on a virDomainpPtr. APIs that are initially within scope of this idea are virDomainMigrate virDomainMigrateToURI virDomainSave virDomainRestore virDomainCoreDump These all take a potentially long time and benefit from monitoring. The virDomainJobInfo struct allows for various pieces of information to be reported - Percentage completion - Time - Overall data - Guest memory data - Guest disk/file data * include/libvirt/libvirt.h.in: Add virDomainGetJobInfo * python/generator.py, python/libvirt-override-api.xml, python/libvirt-override.c: Override for virDomainGetJobInfo API * python/typewrappers.c, python/typewrappers.h: Introduce wrapper for unsigned long long type --- include/libvirt/libvirt.h.in| 49 +++ python/generator.py |1 + python/libvirt-override-api.xml |5 python/libvirt-override.c | 36 python/typewrappers.c |8 ++ python/typewrappers.h |1 + 6 files changed, 100 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 260505e..51c0844 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1787,6 +1787,55 @@ char *virConnectBaselineCPU(virConnectPtr conn, unsigned int ncpus, unsigned int flags); +typedef enum { +VIR_DOMAIN_JOB_NONE = 0, /* No job is active */ +VIR_DOMAIN_JOB_BOUNDED = 1, /* Job with a finite completion time */ +VIR_DOMAIN_JOB_UNBOUNDED = 2, /* Job without a finite completion time */ +VIR_DOMAIN_JOB_COMPLETED = 3, /* Job has finished, but isn't cleaned up */ +VIR_DOMAIN_JOB_FAILED= 4, /* Job hit error, but isn't cleaned up */ 5 disapeared ? Opps, bad counting ! +VIR_DOMAIN_JOB_CANCELLED = 6, /* Job was aborted, but isn't cleaned up */ +} virDomainJobType; + +typedef struct _virDomainJobInfo virDomainJobInfo; +typedef virDomainJobInfo *virDomainJobInfoPtr; +struct _virDomainJobInfo { +/* One of virDomainJobType */ +int type; + +/* Time is measured in seconds */ +unsigned long long timeElapsed;/* Always set */ +unsigned long long timeRemaining; /* Only for VIR_DOMAIN_JOB_BOUNDED */ + +/* Data is measured in bytes unless otherwise specified + * and is measuring the job as a whole + * + * For VIR_DOMAIN_JOB_UNBOUNDED, dataTotal may be less + * than the final sum of dataProcessed + dataRemaining + * in the event that the hypervisor has to repeat some + * data eg due to dirtied pages during migration + * + * For VIR_DOMAIN_JOB_BOUNDED, dataTotal shall always + * equal sum of dataProcessed + dataRemaining + */ +unsigned long long dataTotal; +unsigned long long dataProcessed; +unsigned long long dataRemaining; + +/* As above, but only tracking guest memory progress */ +unsigned long long memTotal; +unsigned long long memProcessed; +unsigned long long memRemaining; + +/* As above, but only tracking guest disk file progress */ +unsigned long long fileTotal; +unsigned long long fileProcessed; +unsigned long long fileRemaining; +}; I wonder if we should not somehow provide an unit for the items being measured. Maybe we don't want bytes in the future The idea here is that the 'mem' and 'file' fields are always in bytes. The 'data' field units are defined by the type of job being executed, though effectively they're always bytes too. +int virDomainGetJobInfo(virDomainPtr dom, +virDomainJobInfoPtr info); + + #ifdef __cplusplus } #endif diff --git a/python/generator.py b/python/generator.py index 24eaf50..f7625fd 100755 --- a/python/generator.py +++ b/python/generator.py @@ -271,6 +271,7 @@ skip_impl = ( 'virConnGetLastError', 'virGetLastError', 'virDomainGetInfo', +'virDomainGetJobInfo', 'virNodeGetInfo', 'virDomainGetUUID', 'virDomainGetUUIDString', diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 76a6fd5..1260c0c 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -48,6 +48,11 @@ return type='int *' info='the list of information or None in case of error'/ arg name='domain' type='virDomainPtr' info='a domain object'/ /function +function name='virDomainGetJobInfo' file='python' + infoExtract information about an
Re: [libvirt] [PATCH 4/6] Fix PCI address handling when controllers are deleted.
On Fri, Feb 26, 2010 at 02:09:17PM +0100, Wolfgang Mauerer wrote: When a controller is not present in the system anymore, the PCI address must be deleted from libvirt's hashtable because it can be re-used for other purposes. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- src/qemu/qemu_driver.c |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5394ff5..8960ef8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6280,6 +6280,11 @@ static int qemudDomainDetachPciControllerDevice(struct qemud_driver *driver, VIR_FREE(vm-def-controllers); vm-def-ncontrollers = 0; } + +if (qemuDomainPCIAddressReleaseAddr(priv-pciaddrs, detach-info) 0) { +VIR_WARN0(Unable to release PCI address on controller); +} + virDomainControllerDefFree(detach); ret = 0; ACK, I have a feeling we leak the address in other hotunplug code too 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 2/6] Always link with libgcrypt
On Fri, Feb 26, 2010 at 02:09:15PM +0100, Wolfgang Mauerer wrote: ... not just when using the remote driver: libvirt.c always depends on it. This unbreaks builds with --without-remote Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- src/Makefile.am |7 --- 1 files changed, 4 insertions(+), 3 deletions(-) Hmm, I think we also need to make the TLS threads code in libvirt.c conditional on WITH_REMOTE, since we don't want that compiled at all if the remote driver isn't present diff --git a/src/Makefile.am b/src/Makefile.am index 2051e5d..8f0fd30 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -334,8 +334,11 @@ libvirt_la_LIBADD += libvirt_driver.la libvirt_driver_la_SOURCES = $(DRIVER_SOURCES) libvirt_driver_la_CFLAGS = $(NUMACTL_CFLAGS) \ + $(GNUTLS_CFLAGS)\ -...@top_srcdir@/src/conf -libvirt_driver_la_LDFLAGS = $(NUMACTL_LIBS) +libvirt_driver_la_LDFLAGS = $(NUMACTL_LIBS) \ + $(GNUTLS_LIBS) + USED_SYM_FILES = libvirt_private.syms @@ -362,11 +365,9 @@ noinst_LTLIBRARIES += libvirt_driver_remote.la libvirt_la_LIBADD += libvirt_driver_remote.la endif libvirt_driver_remote_la_CFLAGS =\ - $(GNUTLS_CFLAGS)\ $(SASL_CFLAGS) \ -...@top_srcdir@/src/conf libvirt_driver_remote_la_LDFLAGS = \ - $(GNUTLS_LIBS) \ $(SASL_LIBS) if WITH_DRIVER_MODULES libvirt_driver_remote_la_LDFLAGS += -module -avoid-version 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] libvirt/dnsmasq integration.
On Wed, Feb 24, 2010 at 04:37:04PM +, Simon Kelley wrote: Richard W.M. Jones wrote: On Wed, Feb 24, 2010 at 02:34:53PM +, Simon Kelley wrote: [...] Does that make sense? It's a long and involved explanation to come to cold. I fear I may have over-simplified what libvirt is doing with dnsmasq, in which case please enlighten me and I'll modify my scheme to take that into account. If this looks good I can easily have the necessary dnsmasq changes in the next release. I read through it and it makes sense to me. The only thing: is /etc/dnsmasq.d a true standard across all distros? It is for Fedora, but an old Debian distro I use does not have this directory. The ability to read config fragments from a directory has been in dnsmasq since version 2.32 (effectively, forever). The Debian (and by inheritance, Ubuntu) packages have created and automatically searched /etc/dnsmasq.d since 2.46, released in November 2008. That's not quite old enough to be in the current Debian stable, which has 2.45. All this is slighly moot, since this scheme needs changes to dnsmasq. Those can easily go into the next release. Is '/etc/init.d/dnsmasq restart' a reliable way to restart the system dnsmasq across all distros? (Perhaps 'service dnsmasq restart' is better?). Almost certainly. Could this be a hook supplied by the distro packaging? I'm not sure that the 'service' command is standardized on all distros either, is it ? Does dnsmasq need a full restart, or is a SIGHUP sufficient ? If the latter, then we could avoid the initscript entirely, by reading dnsmasq's pid file, using kill(0, $PID) and check for -ERSCH to see if it is running or not. Then finally SIGHUP it to reload 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] [macvtap] interface type direct web docs addition
On Tue, Feb 23, 2010 at 12:31:07PM -0500, Stefan Berger wrote: This adds a description about the 'direct' type of interface recently added for macvtap device type support on the host. Index: libvirt-macvtap/docs/formatdomain.html.in === --- libvirt-macvtap.orig/docs/formatdomain.html.in +++ libvirt-macvtap/docs/formatdomain.html.in @@ -674,6 +674,28 @@ .../pre +h5a name=elementsNICSDirectDirect attachment to physical interface/a/h5 + +p + Provides direct attachment of the virtual machine's NIC to the given + physial interface of the host. This setup requires the Linux macvtap + driver to be available. One of the modes 'vepa', 'bridge' or 'private' + can be chosen for the operation mode of the macvtap device, 'vepa' + being the default mode. +/p + +pre + ... + lt;devicesgt; +lt;interface type='direct'/gt; +... +lt;interface type='direct'gt; + lt;source dev='eth0' mode='vepa'/gt; +lt;/interfacegt; + lt;/devicesgt; + .../pre + + h5a name=elementsNICSEthernetGeneric ethernet connection/a/h5 p ACK, though it might be nice to describe the difference between the modes 'vepa', 'bridge' or 'private' for people who are not so familiar with this support 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] Do not search xenstore for disk device IDs
On Tue, Feb 23, 2010 at 03:35:04PM -0700, Jim Fehlig wrote: Daniel P. Berrange wrote: On Mon, Feb 22, 2010 at 02:50:07PM -0700, Jim Fehlig wrote: Disk devices can be referenced by name in Xen, e.g. when modifying their configuration or remvoving them. As such, don't search xenstore for a device ID corresponding to the disk device. Instead, search the disks contained in the domain definition and use the disk's target name if found. This approach allows removing a disk when domain is inactive. We obviously can't search xenstore when the domain is inactive. This sounds reasonable, but I'm wondering if old XenD support the lookup-based on name ? Yes, I had thought about this as well. Guess it depends on how old we are talking about. It exists in Xen 3.1.x and newer. I wonder if we should even care about Xen 3.0.x. The upstream tree hasn't been touched in 3 years. Well we still ship this, so it would be appreciated if it didn't broke if libvirt got updated for some reasons ;-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt/dnsmasq integration.
Daniel P. Berrange wrote: On Wed, Feb 24, 2010 at 04:37:04PM +, Simon Kelley wrote: Richard W.M. Jones wrote: On Wed, Feb 24, 2010 at 02:34:53PM +, Simon Kelley wrote: [...] Does that make sense? It's a long and involved explanation to come to cold. I fear I may have over-simplified what libvirt is doing with dnsmasq, in which case please enlighten me and I'll modify my scheme to take that into account. If this looks good I can easily have the necessary dnsmasq changes in the next release. I read through it and it makes sense to me. The only thing: is /etc/dnsmasq.d a true standard across all distros? It is for Fedora, but an old Debian distro I use does not have this directory. The ability to read config fragments from a directory has been in dnsmasq since version 2.32 (effectively, forever). The Debian (and by inheritance, Ubuntu) packages have created and automatically searched /etc/dnsmasq.d since 2.46, released in November 2008. That's not quite old enough to be in the current Debian stable, which has 2.45. All this is slighly moot, since this scheme needs changes to dnsmasq. Those can easily go into the next release. Is '/etc/init.d/dnsmasq restart' a reliable way to restart the system dnsmasq across all distros? (Perhaps 'service dnsmasq restart' is better?). Almost certainly. Could this be a hook supplied by the distro packaging? I'm not sure that the 'service' command is standardized on all distros either, is it ? I don't know. It exists on recent Debian and Ubuntu versions. Does dnsmasq need a full restart, or is a SIGHUP sufficient ? A full restart: Once running it has dropped root, and some configuration changes need root to establish. There's a subset of the configuration that can be put in special files and reloaded by SIGHUP. If the latter, then we could avoid the initscript entirely, by reading dnsmasq's pid file, using kill(0, $PID) and check for -ERSCH to see if it is running or not. Then finally SIGHUP it to reload Simon. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] Add new clock mode allowing variable adjustments
On Thu, Feb 18, 2010 at 05:54:28PM +, Daniel P. Berrange wrote: This introduces a third option for clock offset synchronization, that allows an arbitrary / variable adjustment to be set. In essence the XML contains the time delta in seconds, relative to UTC. clock offset='variable' adjustment='123465'/ The difference from 'utc' mode, is that management apps should track adjustments and preserve them at next reboot. hum ... if the management layer start to track time of delta w.r.t. UTC as defined by node clock, I also assume they manage the variations in clock when migrating too (or that the nodes clocks are actually properly sync'ed). * docs/schemas/domain.rng: Schema for new clock mode * src/conf/domain_conf.c, src/conf/domain_conf.h: Parse new clock time delta * src/libvirt_private.syms, src/util/xml.c, src/util/xml.h: Add virXPathLongLong() method --- docs/schemas/domain.rng | 25 +--- src/conf/domain_conf.c | 18 +- src/conf/domain_conf.h |5 src/libvirt_private.syms |1 + src/util/xml.c | 54 ++ src/util/xml.h |5 +++- 6 files changed, 101 insertions(+), 7 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 1ff0944..d295bfe 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -297,12 +297,24 @@ define name=clock optional element name=clock -attribute name=offset - choice + choice + attribute name=offset hum isn't that inserting tabs in the XML that we remove from time to time :-) ? valuelocaltime/value + /attribute + attribute name=offset valueutc/value - /choice -/attribute + /attribute + group +attribute name=offset + valuevariable/value + /attribute + optional + attribute name=adjustment + ref name=timeDelta/ + /attribute + /optional + /group + /choice empty/ /element /optional @@ -1567,4 +1579,9 @@ param name='pattern'[a-zA-Z0-9\-_]+/param /data /define + define name=timeDelta +data type=string actually we could use XSD integer there http://www.w3.org/TR/xmlschema-2/#integer but this won't change much + param name=pattern(-|\+)?[0-9]+/param +/data + /define /grammar diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f86b4eb..49d5d19 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -231,7 +231,8 @@ VIR_ENUM_IMPL(virDomainNetdevMacvtap, VIR_DOMAIN_NETDEV_MACVTAP_MODE_LAST, VIR_ENUM_IMPL(virDomainClockOffset, VIR_DOMAIN_CLOCK_OFFSET_LAST, utc, - localtime); + localtime, + variable); #define virDomainReportError(code, fmt...) \ virReportErrorHelper(NULL, VIR_FROM_DOMAIN, code, __FILE__, \ @@ -3492,6 +3493,13 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, } else { def-clock.offset = VIR_DOMAIN_CLOCK_OFFSET_UTC; } +switch (def-clock.offset) { +case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE: +if (virXPathLongLong(./clock/@adjustment, ctxt, + def-clock.adjustment) 0) +def-clock.adjustment = 0; Hum, should probably give an error message here +break; +} def-os.bootloader = virXPathString(string(./bootloader), ctxt); def-os.bootloaderArgs = virXPathString(string(./bootloader_args), ctxt); @@ -5399,8 +5407,14 @@ char *virDomainDefFormat(virDomainDefPtr def, if (virCPUDefFormatBuf(buf, def-cpu, , 0) 0) goto cleanup; -virBufferVSprintf(buf, clock offset='%s'/\n, +virBufferVSprintf(buf, clock offset='%s', virDomainClockOffsetTypeToString(def-clock.offset)); +switch (def-clock.offset) { +case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE: +virBufferVSprintf(buf, adjustment='%lld', def-clock.adjustment); +break; +} +virBufferAddLit(buf, /\n); if (virDomainLifecycleDefFormat(buf, def-onPoweroff, on_poweroff) 0) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index fbbe683..f5fe016 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -612,6 +612,7 @@ struct _virSecurityLabelDef { enum virDomainClockOffsetType { VIR_DOMAIN_CLOCK_OFFSET_UTC = 0, VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME = 1, +VIR_DOMAIN_CLOCK_OFFSET_VARIABLE = 2, VIR_DOMAIN_CLOCK_OFFSET_LAST, }; @@ -620,6 +621,10 @@ typedef struct _virDomainClockDef virDomainClockDef; typedef virDomainClockDef *virDomainClockDefPtr; struct _virDomainClockDef { int offset; + +
Re: [libvirt] [PATCH 3/6] Support variable clock offset mode in QEMU
On Thu, Feb 18, 2010 at 05:54:29PM +, Daniel P. Berrange wrote: This allows QEMU guests to be started with an arbitrary clock offset The test case can't actually be enabled, since QEMU argv expects an absolute timestring, and this will obviously change every time the test runs :-( Hopefully QEMU will allow a relative time offset in the future. * src/qemu/qemu_conf.c, src/qemu/qemu_conf.h: Use the -rtc arg if available to support variable clock offset mode * tests/qemuhelptest.c: Add QEMUD_CMD_FLAG_RTC for qemu 0.12.1 * qemuxml2argvdata/qemuxml2argv-clock-variable.args, qemuxml2argvdata/qemuxml2argv-clock-variable.xml, qemuxml2argvtest.c: Test case, except we can't actually enable it yet. --- src/qemu/qemu_conf.c | 82 ++-- src/qemu/qemu_conf.h |1 + tests/qemuhelptest.c |3 +- .../qemuxml2argv-clock-variable.args |1 + .../qemuxml2argv-clock-variable.xml| 24 ++ tests/qemuxml2argvtest.c |4 + 6 files changed, 107 insertions(+), 8 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-clock-variable.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-clock-variable.xml diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index a207fc7..112a7c2 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1155,6 +1155,9 @@ static unsigned long long qemudComputeCmdFlags(const char *help, flags |= QEMUD_CMD_FLAG_BALLOON; if (strstr(help, -device)) flags |= QEMUD_CMD_FLAG_DEVICE; +/* The trailing ' ' is important to avoid a bogus match */ +if (strstr(help, -rtc )) +flags |= QEMUD_CMD_FLAG_RTC; /* Keep disabled till we're actually ready to turn on netdev mode * The plan is todo it in 0.13.0 QEMU, but lets wait see... */ #if 0 @@ -2969,6 +2972,56 @@ error: } +static char * +qemuBuildClockArgStr(virDomainClockDefPtr def) +{ +virBuffer buf = VIR_BUFFER_INITIALIZER; + +switch (def-offset) { +case VIR_DOMAIN_CLOCK_OFFSET_UTC: +virBufferAddLit(buf, base=utc); +break; + +case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME: +virBufferAddLit(buf, base=localtime); +break; + +case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE: { +time_t now = time(NULL); +struct tm nowbits; + +now += def-adjustment; +gmtime_r(now, nowbits); + +virBufferVSprintf(buf, base=%d-%d-%dT%d:%d:%d, + nowbits.tm_year, + nowbits.tm_mon, + nowbits.tm_mday, + nowbits.tm_hour, + nowbits.tm_min, + nowbits.tm_sec); +} break; + +default: +qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, +_(unsupported clock offset '%s'), +virDomainClockOffsetTypeToString(def-offset)); +goto error; +} + +if (virBufferError(buf)) { +virReportOOMError(); +goto error; +} + +return virBufferContentAndReset(buf); + +error: +virBufferFreeAndReset(buf); +return NULL; +} + + static int qemuBuildCpuArgStr(const struct qemud_driver *driver, const virDomainDefPtr def, @@ -3400,13 +3453,28 @@ int qemudBuildCommandLine(virConnectPtr conn, } } -if (def-clock.offset == VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME) -ADD_ARG_LIT(-localtime); -else if (def-clock.offset != VIR_DOMAIN_CLOCK_OFFSET_UTC) { -qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, -_(unsupported clock offset '%s'), -virDomainClockOffsetTypeToString(def-clock.offset)); -goto error; +if (qemuCmdFlags QEMUD_CMD_FLAG_RTC) { +const char *rtcopt; +ADD_ARG_LIT(-rtc); +if (!(rtcopt = qemuBuildClockArgStr(def-clock))) +goto error; +ADD_ARG(rtcopt); +} else { +switch (def-clock.offset) { +case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME: +ADD_ARG_LIT(-localtime); +break; + +case VIR_DOMAIN_CLOCK_OFFSET_UTC: +/* Nothing, its the default */ +break; + +default: +qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, +_(unsupported clock offset '%s'), + virDomainClockOffsetTypeToString(def-clock.offset)); +goto error; +} } if ((qemuCmdFlags QEMUD_CMD_FLAG_NO_REBOOT) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 7041489..e32a3d7 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -82,6 +82,7 @@ enum qemud_cmd_flags { QEMUD_CMD_FLAG_SDL =
Re: [libvirt] [PATCH 5/6] Allow configurable timezones with QEMU
On Thu, Feb 18, 2010 at 05:54:31PM +, Daniel P. Berrange wrote: Allow an arbitrary timezone with QEMU by setting the $TZ environment variable when launching QEMU * src/qemu/qemu_conf.c: Set TZ environment variable if a timezone is requested * tests/qemuxml2argvtest.c: Add test case for timezones * tests/qemuxml2argvdata/qemuxml2argv-clock-france.xml, tests/qemuxml2argvdata/qemuxml2argv-clock-france.args: Data for timezone tests --- src/qemu/qemu_conf.c |6 + .../qemuxml2argv-clock-france.args |1 + .../qemuxml2argvdata/qemuxml2argv-clock-france.xml | 24 tests/qemuxml2argvtest.c |2 + 4 files changed, 33 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-clock-france.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-clock-france.xml diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index cd0dd7f..4ed00cb 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -2983,6 +2983,7 @@ qemuBuildClockArgStr(virDomainClockDefPtr def) break; case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME: +case VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE: virBufferAddLit(buf, base=localtime); break; @@ -3462,6 +3463,7 @@ int qemudBuildCommandLine(virConnectPtr conn, } else { switch (def-clock.offset) { case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME: +case VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE: ADD_ARG_LIT(-localtime); break; @@ -3476,6 +3478,10 @@ int qemudBuildCommandLine(virConnectPtr conn, goto error; } } +if (def-clock.offset == VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE +def-clock.data.timezone) { +ADD_ENV_PAIR(TZ, def-clock.data.timezone); +} if ((qemuCmdFlags QEMUD_CMD_FLAG_NO_REBOOT) def-onReboot != VIR_DOMAIN_LIFECYCLE_RESTART) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-clock-france.args b/tests/qemuxml2argvdata/qemuxml2argv-clock-france.args new file mode 100644 index 000..3c57397 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-clock-france.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test TZ=Europe/Paris /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait -rtc base=localtime -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-clock-france.xml b/tests/qemuxml2argvdata/qemuxml2argv-clock-france.xml new file mode 100644 index 000..157fdfb --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-clock-france.xml @@ -0,0 +1,24 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory219200/memory + currentMemory219200/currentMemory + vcpu1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='timezone' timezone='Europe/Paris'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='block' device='disk' + source dev='/dev/HostVG/QEMUGuest1'/ + target dev='hda' bus='ide'/ + address type='drive' controller='0' bus='0' unit='0'/ +/disk +controller type='ide' index='0'/ + /devices +/domain diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 510176c..bd41b6c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -230,6 +230,8 @@ mymain(int argc, char **argv) * Can't be enabled since the absolute timestamp changes every time DO_TEST(clock-variable, QEMUD_CMD_FLAG_RTC); */ +DO_TEST(clock-france, QEMUD_CMD_FLAG_RTC); + DO_TEST(hugepages, QEMUD_CMD_FLAG_MEM_PATH); DO_TEST(disk-cdrom, 0); DO_TEST(disk-cdrom-empty, QEMUD_CMD_FLAG_DRIVE); ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Do not search xenstore for disk device IDs
On Mon, Mar 01, 2010 at 02:45:41PM +0100, Daniel Veillard wrote: On Tue, Feb 23, 2010 at 03:35:04PM -0700, Jim Fehlig wrote: Daniel P. Berrange wrote: On Mon, Feb 22, 2010 at 02:50:07PM -0700, Jim Fehlig wrote: Disk devices can be referenced by name in Xen, e.g. when modifying their configuration or remvoving them. As such, don't search xenstore for a device ID corresponding to the disk device. Instead, search the disks contained in the domain definition and use the disk's target name if found. This approach allows removing a disk when domain is inactive. We obviously can't search xenstore when the domain is inactive. This sounds reasonable, but I'm wondering if old XenD support the lookup-based on name ? Yes, I had thought about this as well. Guess it depends on how old we are talking about. It exists in Xen 3.1.x and newer. I wonder if we should even care about Xen 3.0.x. The upstream tree hasn't been touched in 3 years. Well we still ship this, so it would be appreciated if it didn't broke if libvirt got updated for some reasons ;-) Yep, RHEL-5 still ships with Xen 3.0.x series we aim to support that in libvirt. I think we can probably just make your new approach be conditional on the xendConfigVersion variable we already have. 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] qemu: avoid null dereference on failed migration
On Fri, Feb 26, 2010 at 10:17:53AM -0700, Eric Blake wrote: From: Eric Blake ebl...@redhat.com * src/qemu/qemu_monitor_text.c (qemuMonitorTextGetMigrationStatus): Check for failed strchr, to silence a coverity warning. Signed-off-by: Eric Blake ebl...@redhat.com --- src/qemu/qemu_monitor_text.c |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 62ffcc6..e7b4b1f 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -989,6 +989,11 @@ int qemuMonitorTextGetMigrationStatus(qemuMonitorPtr mon, if ((tmp = strstr(reply, MIGRATION_PREFIX)) != NULL) { tmp += strlen(MIGRATION_PREFIX); end = strchr(tmp, '\r'); +if (end == NULL) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, +_(unexpected migration status in %s), reply); +goto cleanup; +} *end = '\0'; if ((*status = qemuMonitorMigrationStatusTypeFromString(tmp)) 0) { ACK, applied, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/6] Implement hotremove for SCSI disks
On 03/01/10 15:29, Wolfgang Mauerer wrote: Gerd: Are you intending to add the drive_del feature, or is the approach outlined above sufficient for drive hotplug/remove? I'm busy with other tasks right now. Respectively can there be any problems if we remove a device associated with a disk and then re-create a drive/device pair with the same IDs as before, but with a different configuration? There are no problems with that, when it is gone from 'info block' all traces of the drive are gone and creating another one with the same name is not a problem. For symmetry reasons it would be nice to have a drive_del command though and have device_del not implicitly zap the drive. The current scheme also fails to handle some corner cases like device_add failing (you are left with a drive you can't remove easily). cheers, Gerd -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] cmdPoolDiscoverSources: initialize earlier to avoid FP from clang
This change hoists the initialization of srcSpec to the top, to keep clang from reporting a false-positive used-uninitialized problem. From f487655124c887310be3bc82d7beae3cd47e7d21 Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Mon, 1 Mar 2010 15:41:15 +0100 Subject: [PATCH] cmdPoolDiscoverSources: initialize earlier to avoid FP from clang * tools/virsh.c (cmdPoolDiscoverSources): Always initialize srcSpec. Otherwise, clang would report that srcSpec could be used uninitialized in the call to virConnectFindStoragePoolSources. --- tools/virsh.c |7 +++ 1 files changed, 3 insertions(+), 4 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 3b1011c..191e7ba 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -4431,17 +4431,16 @@ static const vshCmdOptDef opts_find_storage_pool_sources[] = { static int cmdPoolDiscoverSources(vshControl * ctl, const vshCmd * cmd ATTRIBUTE_UNUSED) { -char *type, *srcSpec, *srcSpecFile, *srcList; +char *type, *srcSpecFile, *srcList; +char *srcSpec = NULL; int found; type = vshCommandOptString(cmd, type, found); if (!found) return FALSE; srcSpecFile = vshCommandOptString(cmd, srcSpec, found); -if (!found) { +if (!found) srcSpecFile = NULL; -srcSpec = NULL; -} if (!vshConnectionUsability(ctl, ctl-conn, TRUE)) return FALSE; -- 1.7.0.1.414.g89213d -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt/dnsmasq integration.
On Mon, Mar 01, 2010 at 01:54:44PM +, Simon Kelley wrote: Daniel P. Berrange wrote: I imagine you've already seen this, but as an example of the ARGV that libvirt generates for its dnsmasq instances: /usr/sbin/dnsmasq \ --strict-order \ --bind-interfaces \ --pid-file=/var/run/libvirt/network/default.pid \ --conf-file= \ --listen-address 192.168.122.1 \ --except-interface lo \ --dhcp-range 192.168.122.2,192.168.122.254 \ --dhcp-lease-max=253 That's useful to know. For new releases, --dhcp-lease-max is no longer needed,(the default changed from 150 to 1000). Why are you using --strict-order? That's not normally a sensible choice. We had bug reports from users that VMs weren't doing DNS resolution in the same way as apps on the host. We determined that GLibc's resolver was applying strict ordering, thus enabling --strict-order in dnsmasq gives us consistent behaviour. My other concern with writing libvirt configs into /etc/dnsmasq.d is that users will then get the impression that this is something that they can freely edit / modify at will. They'll be unhappy with libvirt overwrites their changes whenever it starts. This could perhaps be addressed by allowing use to put the configs into /var/lib/dnsmasq/ instead of /etc/dnsmasq.d, which is more common location for non-user editable configs generated at runtime. That's a packaging, rather then dnsmasq, issue: /etc/dnsmasq.d is not hard-coded into the binary, it's supplied on the command line by the start-up script. (again this is for Debian and Ubuntu). There's no reason why /var/lib/dnsmasq/ couldn't be added too. The downside of that would be more that it's more mysterious for users trying to work out exactly where all this configuration is coming from. Maybe a good compromise might be a DO NOT EDIT header in /etc/dnsmasq.d/libvirt. Ok, that's good to know Your general plan of having a single dnsmasq instance though does sound desirable, given the way the sockets() APIs work wrt binding to addresses The total set of DNSMASQ args that we currently use are --strict-order --bind-interfaces --domain DOMAIN-NAME(optional) --pid-file=/var/run/libvirt/network/$NETWORK.pid --conf-file= --listen-address=IPADDR-OF-BRIDGE --except-interface=lo --dhcp-range=IPRANGE(optional, multiple times) --dhcp-lease-max=RANGE-SIZE (optional) --dhcp-host=STATIC-HOST-MAPPING (optional) --enable-tftp (optional) --tftp-root=/some/path (optional) --dhcp-boot=PXE-BOOT-SERVER (optional) OK, the use of TFTP by libvirt is new to me. Looks like I'll have to pull some similar tricks to allow libvirt to enable TFTP on on interface without disturbing things elsewhere. We only very recently added the TFTP/boot options so it is probably not visible in many Linux distros yet. NB, we explicitly give a NULL conf-file in order to prevent any of the user's settings from the system instance from conflicting with libvirts settings. We don't really want users to be able to specify arbitrary other configuration settings for the libvirt dnsmasq instances, other than those we enable via the libvirt XML configuration. I've used 'optional' to denote flags we only pass when explicitly configured via libvirt's XML format. The others we pass all the time. The 'lease-max' arg we calculate to be exactly matching the number of addresses in the configured dhcp-range args. This is because some of our users had configured dhcp ranges larger than 150 addresses in len. See comment earlier - the default has gone up to 1000, so you may not need this now. Yep, that sounds pretty reasonable. Would this scheme allow libvirt to guarantee that no DHCP is present on its interface ? We currently support running in DNS-only mode, or DNS+DHCP. It is desirable to keep that regardless of how the host's system dnsmasq is currently configured for other interfaces. If I am understanding your suggestion, this allows libvirt to easily enable DNS+DHCP mode on its own interface, without us accidentally enabling DHCP on other host interfaces. If libvirt doesn't use any --dhcp-range flags, there is still a chance that DHCP could be enabled on libvirt's interface if the system dnsmasq had any dhcp-range args. Though assuming the IP ranges don't overlap this should be effectively a no-op ? Just emit --no-dhcp-interface=virb0 instead of a dhcp-range. That solves that problem. Ok I think you've got the general picture of what we're doing with dnsmasq. At a very high level our original goals were - Support multiple independantly configured networks (virbr0, virbr1, etc) - Isolation between libvirt network interface config host inteface config - Only support configuratin of options via
Re: [libvirt] [PATCH 6/6] Implement hotremove for SCSI disks
On Mon, Mar 01, 2010 at 03:40:14PM +0100, Gerd Hoffmann wrote: On 03/01/10 15:29, Wolfgang Mauerer wrote: Gerd: Are you intending to add the drive_del feature, or is the approach outlined above sufficient for drive hotplug/remove? I'm busy with other tasks right now. Respectively can there be any problems if we remove a device associated with a disk and then re-create a drive/device pair with the same IDs as before, but with a different configuration? There are no problems with that, when it is gone from 'info block' all traces of the drive are gone and creating another one with the same name is not a problem. Great, this is sufficient for the hot-unplug code to work. For symmetry reasons it would be nice to have a drive_del command though and have device_del not implicitly zap the drive. The current scheme also fails to handle some corner cases like device_add failing (you are left with a drive you can't remove easily). Yep, that's a minor edge case we can worry about later 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
[libvirt] [RFC PATCH] build: consistently use C99 varargs macros
Prior to this patch, there was an inconsistent mix between GNU and C99. For consistency, and potential portability to other compilers, stick with the C99 vararg macro syntax. * src/conf/cpu_conf.c (virCPUReportError): Use C99 rather than GNU vararg macro syntax. * src/conf/domain_conf.c (virDomainReportError): Likewise. * src/conf/domain_event.c (eventReportError): Likewise. * src/conf/interface_conf.c (virInterfaceReportError): Likewise. * src/conf/network_conf.c (virNetworkReportError): Likewise. * src/conf/node_device_conf.h (virNodeDeviceReportError): Likewise. * src/conf/secret_conf.h (virSecretReportError): Likewise. * src/conf/storage_conf.h (virStorageReportError): Likewise. --- Any objections to expanding this into a full-blown patch series? This touches 8 files; I counted 43 more files that could be altered, using: $ git grep -l 'define.*[a-zA-Z]\.\.\.' src/conf/cpu_conf.c |4 ++-- src/conf/domain_conf.c |4 ++-- src/conf/domain_event.c |5 +++-- src/conf/interface_conf.c |4 ++-- src/conf/network_conf.c |6 +++--- src/conf/node_device_conf.h |5 +++-- src/conf/secret_conf.h |6 +++--- src/conf/storage_conf.h |6 +++--- 8 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index ed83188..612e376 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -31,9 +31,9 @@ #define VIR_FROM_THIS VIR_FROM_CPU -#define virCPUReportError(code, fmt...) \ +#define virCPUReportError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_CPU, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) VIR_ENUM_IMPL(virCPUMatch, VIR_CPU_MATCH_LAST, minimum, diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4420445..ea84863 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -232,9 +232,9 @@ VIR_ENUM_IMPL(virDomainNetdevMacvtap, VIR_DOMAIN_NETDEV_MACVTAP_MODE_LAST, private, bridge) -#define virDomainReportError(code, fmt...) \ +#define virDomainReportError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_DOMAIN, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) #ifndef PROXY diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index b474b1c..e791cab 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -1,6 +1,7 @@ /* * domain_event.c: domain event queue processing helpers * + * Copyright (C) 2010 Red Hat, Inc. * Copyright (C) 2008 VirtualIron * * This library is free software; you can redistribute it and/or @@ -30,9 +31,9 @@ #define VIR_FROM_THIS VIR_FROM_NONE -#define eventReportError(conn, code, fmt...)\ +#define eventReportError(conn, code, ...) \ virReportErrorHelper(conn, VIR_FROM_THIS, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) /** diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index a0d2dfa..bc9e958 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -45,9 +45,9 @@ static int virInterfaceDefDevFormat(virBufferPtr buf, const virInterfaceDefPtr def, int level); -#define virInterfaceReportError(code, fmt...) \ +#define virInterfaceReportError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_INTERFACE, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) static void virInterfaceIpDefFree(virInterfaceIpDefPtr def) { diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 6d3c3c0..39ebd62 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1,7 +1,7 @@ /* * network_conf.c: network XML handling * - * Copyright (C) 2006-2009 Red Hat, Inc. + * Copyright (C) 2006-2010 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -53,9 +53,9 @@ VIR_ENUM_IMPL(virNetworkForward, VIR_NETWORK_FORWARD_LAST, none, nat, route ) -#define virNetworkReportError(code, fmt...) \ +#define virNetworkReportError(code, ...)\ virReportErrorHelper(NULL, VIR_FROM_NETWORK, code, __FILE__,\ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) virNetworkObjPtr virNetworkFindByUUID(const virNetworkObjListPtr nets, const
Re: [libvirt] [PATCH 0/3] Refactor XML parsing code
On Thu, Feb 25, 2010 at 01:12:47PM +0100, Daniel Veillard wrote: On Wed, Feb 24, 2010 at 11:07:31PM +0100, Jiri Denemark wrote: Almost identical XML parsing code for either strings or files was copied 15 times. Since it was needed on two other places for parsing CPU XML strings with correct error reporting, I decided to factor the parsing code out to xml.c and used this one implementation in all other places. looking very quickly, this looks sensible, but I can't fully review this right now, ACK on principle though, maybe this can wait for 0.7.8. Okay, reread them, looks just fine by me, ACK, let's push them once 0.7.7 is out :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemudNetworkIfaceConnect: remove dead store
According to Jim Meyering on 3/1/2010 8:06 AM: clang reported this dead store. @@ -1520,11 +1520,10 @@ qemudNetworkIfaceConnect(virConnectPtr conn, errobj = virSaveLastError(); virNetworkFree(network); virSetError(errobj); virFreeError(errobj); -errobj = virSaveLastError(); ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] udevEnumerateDevices: remove dead code
On 03/01/2010 10:15 AM, Jim Meyering wrote: Clang reported the dead store. From 0557e23ca2f5eb78ed9f260040895011785f670f Mon Sep 17 00:00:00 2001 From: Jim Meyeringmeyer...@redhat.com Date: Mon, 1 Mar 2010 16:14:35 +0100 Subject: [PATCH] udevEnumerateDevices: remove dead code * src/node_device/node_device_udev.c (udevEnumerateDevices): Remove unnecessary call to udev_list_entry_get_name. --- src/node_device/node_device_udev.c |4 +--- 1 files changed, 1 insertions(+), 3 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 68509f4..eee44c4 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1,11 +1,11 @@ /* * node_device_udev.c: node device enumeration - libudev implementation * - * Copyright (C) 2009 Red Hat + * Copyright (C) 2009-2010 Red Hat * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public * License as published by the Free Software Foundation; either * version 2.1 of the License, or (at your option) any later version. * * This library is distributed in the hope that it will be useful, @@ -1333,30 +1333,28 @@ static int udevProcessDeviceListEntry(struct udev *udev, } static int udevEnumerateDevices(struct udev *udev) { struct udev_enumerate *udev_enumerate = NULL; struct udev_list_entry *list_entry = NULL; -const char *name = NULL; int ret = 0; udev_enumerate = udev_enumerate_new(udev); ret = udev_enumerate_scan_devices(udev_enumerate); if (0 != ret) { VIR_ERROR(udev scan devices returned %d, ret); goto out; } udev_list_entry_foreach(list_entry, udev_enumerate_get_list_entry(udev_enumerate)) { udevProcessDeviceListEntry(udev, list_entry); -name = udev_list_entry_get_name(list_entry); } out: udev_enumerate_unref(udev_enumerate); return ret; } -- 1.7.0.1.414.g89213d -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemudNetworkIfaceConnect: remove dead store
Eric Blake wrote: According to Jim Meyering on 3/1/2010 8:06 AM: clang reported this dead store. @@ -1520,11 +1520,10 @@ qemudNetworkIfaceConnect(virConnectPtr conn, errobj = virSaveLastError(); virNetworkFree(network); virSetError(errobj); virFreeError(errobj); -errobj = virSaveLastError(); ACK. Thanks for the quick reviews. I'll push as soon as make-distcheck-after-rebase passes. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] Make domain save work on root-squash NFS
Move *all* file operations related to creation and writing of libvirt header to the domain save file into a hook function that is called by virFileOperation. First try to call virFileOperation as root. If that fails with EACCESS, and (in the case of Linux) statfs says that we're trying to save the file on an NFS share, rerun virFileOperation, telling it to fork a child process and setuid to the qemu user. This is the only way we can successfully create a file on a root-squashed NFS server. This patch (along with setting dynamic_ownership=0 in qemu.conf) makes qemudDomainSave work on root-squashed NFS. --- Note that this version of the patch avoids a problem the original had with NFS shares whose directories aren't even readable by root - in those cases, statfs would fail, so we would never learn that the file was on NFS, and just fail the entire operation. The solution is to repeatedly call statfs on a smaller and smaller part of the full path until it succeeds - this will at most continue until the mount point of the share, which will properly report the fstype for the share, rather than for the mount point itself. src/qemu/qemu_driver.c | 165 1 files changed, 139 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1e4b493..da92cf3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -47,6 +47,11 @@ #include sys/ioctl.h #include sys/un.h +#ifdef __linux__ +#include sys/vfs.h +#include linux/magic.h +#endif + #include virterror_internal.h #include logging.h #include datatypes.h @@ -3956,14 +3961,44 @@ struct qemud_save_header { int unused[15]; }; +struct fileOpHookData { +virDomainPtr dom; +const char *path; +char *xml; +struct qemud_save_header *header; +}; + +static int qemudDomainSaveFileOpHook(int fd, void *data) { +struct fileOpHookData *hdata = data; +int ret = 0; + +if (safewrite(fd, hdata-header, sizeof(*hdata-header)) != sizeof(*hdata-header)) { +ret = errno; +qemuReportError(VIR_ERR_OPERATION_FAILED, + _(failed to write save header to '%s'), hdata-path); +goto endjob; +} + +if (safewrite(fd, hdata-xml, hdata-header-xml_len) != hdata-header-xml_len) { +ret = errno; +qemuReportError(VIR_ERR_OPERATION_FAILED, + _(failed to write xml to '%s'), hdata-path); +goto endjob; +} +endjob: +return ret; +} + + static int qemudDomainSave(virDomainPtr dom, const char *path) { struct qemud_driver *driver = dom-conn-privateData; virDomainObjPtr vm = NULL; -int fd = -1; char *xml = NULL; struct qemud_save_header header; +struct fileOpHookData hdata; +int bypassSecurityDriver = 0; int ret = -1; int rc; virDomainEventPtr event = NULL; @@ -4027,34 +4062,113 @@ static int qemudDomainSave(virDomainPtr dom, } header.xml_len = strlen(xml) + 1; +/* Setup hook data needed by virFileOperation hook function */ +hdata.dom = dom; +hdata.path = path; +hdata.xml = xml; +hdata.header = header; + /* Write header to file, followed by XML */ -if ((fd = open(path, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) 0) { -qemuReportError(VIR_ERR_OPERATION_FAILED, -_(failed to create '%s'), path); -goto endjob; -} -if (safewrite(fd, header, sizeof(header)) != sizeof(header)) { -qemuReportError(VIR_ERR_OPERATION_FAILED, -%s, _(failed to write save header)); -goto endjob; -} +/* First try creating the file as root */ +if ((rc = virFileOperation(path, O_CREAT|O_TRUNC|O_WRONLY, + S_IRUSR|S_IWUSR, + getuid(), getgid(), + qemudDomainSaveFileOpHook, hdata, + 0)) != 0) { + +/* If we failed as root, and the error was permission-denied + (EACCES), assume it's on a network-connected share where + root access is restricted (eg, root-squashed NFS). If the + qemu user (driver-user) is non-root, just set a flag to + bypass security driver shenanigans, and retry the operation + after doing setuid to qemu user */ + +if ((rc != EACCES) || +driver-user == getuid()) { +virReportSystemError(rc, _(Failed to create domain save file '%s'), + path); +goto endjob; +} -if (safewrite(fd, xml, header.xml_len) != header.xml_len) { -qemuReportError(VIR_ERR_OPERATION_FAILED, - %s, _(failed to write xml)); -goto endjob; -} +#ifdef __linux__ +/* On Linux we can also verify the FS-type of the directory. */ +char *dirpath, *p; +struct statfs st; +int
Re: [libvirt] [RFC] Proposal for introduction of network traffic filtering capabilities for filtering of network traffic from and to VMs
On Mon, Feb 22, 2010 at 01:45:20PM +0100, Gerhard Stenzel wrote: Hi, here is a preview of a chapter which is eventually intended for the libvirt application development guide. It is not final yet, but I feel now would be a good moment to gather some first feedback and to finalise the XML schema which is used in the examples. Thanks, this is a good idea ! 1. Network Filter 1.1. Overview 1.2. XML Filter Description Format 1.2.1. Complex Filter 1.2.2. Simple Filters 1.3. Retrieving Information About Filter 1.3.1. TBD Chapter 1. Network Filter --- 1.1. Overview 1.2. XML Filter Description Format 1.2.1. Complex Filter 1.2.2. Simple Filters 1.3. Retrieving Information About Filter 1.3.1. TBD This section covers the management and definition of network filters using the libvirt API. 1.2.2. Simple Filters The following examples of simple filters are predefined and address distint filter requirements. The predefined no-arp-spoofing filter drops all ARP packets * originating from the guest if they contain other than the guests IP or MAC address * destined for the guest if they contain other than the guests IP or MAC address It accepts all request or reply ARP packets. filter name='no-arp-spoofing' chain='arp' Perhaps we should call that 'chain' attribute 'protocol' instead since that appears to be what you're representing there. I'm wondering how this should interact with the filterref element. eg, you might have chain='ipv4' on the main filter, and then a filterref pointing to a chain='arp'. One way would be to declare that a filter can contain either rule or filterref, but not a mixture of both. uuidf88f1932-debf-4aa1-9fbe-f10d3aa4bc95/uuid !-- no arp spoofing -- !-- drop if ipaddr or macaddr does not belong to guest -- rule action='drop' direction='out' arp match='no' srcmacaddr='$MAC'/ /rule rule action='drop' direction='out' arp match='no' srcipaddr='$IP' / /rule !-- drop if ipaddr or macaddr odes not belong to guest -- rule action='drop' direction='in' arp match='no' dstmacaddr='$MAC'/ /rule rule action='drop' direction='in' arp match='no' dstipaddr='$IP' / /rule !-- accept only request or reply packets -- rule action='accept' direction='inout' arp opcode='request'/ /rule rule action='accept' direction='inout' arp opcode='reply'/ /rule !-- drop everything else -- rule action='drop' direction='inout'/ /filter Generally, your proposal looks good to me. 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
[libvirt] [PATCH] add an assert, to avoid a false-positive NULL-deref warning from clang
Here's a case in which using an assertion appears to be the only way to tell clang that client really is non-NULL at that point. I'm sure clang's analyzers will eventually improve, and hence avoid this sort of false positive, so have marked this with a FIXME comment, to help ensure we eventually remove this otherwise unnecessary assertion. From ba26bad7aec8713ded0fd3300e951eac3673cc72 Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Mon, 1 Mar 2010 15:19:48 +0100 Subject: [PATCH] add an assert, to avoid a false-positive NULL-deref warning from clang * daemon/libvirtd.c (qemudWorker): Assert. --- daemon/libvirtd.c |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 9bdbecb..a357914 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -24,34 +24,35 @@ #include config.h #include sys/types.h #include sys/wait.h #include sys/stat.h #include unistd.h #include fcntl.h #include limits.h #include sys/socket.h #include sys/un.h #include sys/poll.h #include netinet/in.h #include netinet/tcp.h #include netdb.h #include stdlib.h #include pwd.h #include stdio.h +#include assert.h #include stdarg.h #include syslog.h #include string.h #include errno.h #include getopt.h #include fnmatch.h #include grp.h #include signal.h #include netdb.h #include libvirt_internal.h #include virterror_internal.h #define VIR_FROM_THIS VIR_FROM_QEMU #include libvirtd.h #include dispatch.h @@ -1504,34 +1505,38 @@ static void *qemudWorker(void *data) virMutexLock(server-lock); while (((client = qemudPendingJob(server)) == NULL) !worker-quitRequest) { if (virCondWait(server-job, server-lock) 0) { virMutexUnlock(server-lock); return NULL; } } if (worker-quitRequest) { if (client) virMutexUnlock(client-lock); virMutexUnlock(server-lock); return NULL; } worker-processingCall = 1; virMutexUnlock(server-lock); +/* Tell clang we know what we're doing. + FIXME: remove when clang improves. */ +assert (client); + /* We own a locked client now... */ client-refs++; /* Remove our message from dispatch queue while we use it */ msg = qemudClientMessageQueueServe(client-dx); /* This function drops the lock during dispatch, * and re-acquires it before returning */ if (remoteDispatchClientRequest (server, client, msg) 0) { VIR_FREE(msg); qemudDispatchClientFailure(client); client-refs--; virMutexUnlock(client-lock); continue; } client-refs--; -- 1.7.0.1.414.g89213d -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH] build: consistently use C99 varargs macros
On Mon, Mar 01, 2010 at 08:04:35AM -0700, Eric Blake wrote: Prior to this patch, there was an inconsistent mix between GNU and C99. For consistency, and potential portability to other compilers, stick with the C99 vararg macro syntax. * src/conf/cpu_conf.c (virCPUReportError): Use C99 rather than GNU vararg macro syntax. * src/conf/domain_conf.c (virDomainReportError): Likewise. * src/conf/domain_event.c (eventReportError): Likewise. * src/conf/interface_conf.c (virInterfaceReportError): Likewise. * src/conf/network_conf.c (virNetworkReportError): Likewise. * src/conf/node_device_conf.h (virNodeDeviceReportError): Likewise. * src/conf/secret_conf.h (virSecretReportError): Likewise. * src/conf/storage_conf.h (virStorageReportError): Likewise. --- Any objections to expanding this into a full-blown patch series? This touches 8 files; I counted 43 more files that could be altered, using: $ git grep -l 'define.*[a-zA-Z]\.\.\.' src/conf/cpu_conf.c |4 ++-- src/conf/domain_conf.c |4 ++-- src/conf/domain_event.c |5 +++-- src/conf/interface_conf.c |4 ++-- src/conf/network_conf.c |6 +++--- src/conf/node_device_conf.h |5 +++-- src/conf/secret_conf.h |6 +++--- src/conf/storage_conf.h |6 +++--- 8 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index ed83188..612e376 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -31,9 +31,9 @@ #define VIR_FROM_THIS VIR_FROM_CPU -#define virCPUReportError(code, fmt...) \ +#define virCPUReportError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_CPU, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) VIR_ENUM_IMPL(virCPUMatch, VIR_CPU_MATCH_LAST, minimum, diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4420445..ea84863 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -232,9 +232,9 @@ VIR_ENUM_IMPL(virDomainNetdevMacvtap, VIR_DOMAIN_NETDEV_MACVTAP_MODE_LAST, private, bridge) -#define virDomainReportError(code, fmt...) \ +#define virDomainReportError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_DOMAIN, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) #ifndef PROXY diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index b474b1c..e791cab 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -1,6 +1,7 @@ /* * domain_event.c: domain event queue processing helpers * + * Copyright (C) 2010 Red Hat, Inc. * Copyright (C) 2008 VirtualIron * * This library is free software; you can redistribute it and/or @@ -30,9 +31,9 @@ #define VIR_FROM_THIS VIR_FROM_NONE -#define eventReportError(conn, code, fmt...)\ +#define eventReportError(conn, code, ...) \ virReportErrorHelper(conn, VIR_FROM_THIS, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) /** diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index a0d2dfa..bc9e958 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -45,9 +45,9 @@ static int virInterfaceDefDevFormat(virBufferPtr buf, const virInterfaceDefPtr def, int level); -#define virInterfaceReportError(code, fmt...) \ +#define virInterfaceReportError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_INTERFACE, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) static void virInterfaceIpDefFree(virInterfaceIpDefPtr def) { diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 6d3c3c0..39ebd62 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1,7 +1,7 @@ /* * network_conf.c: network XML handling * - * Copyright (C) 2006-2009 Red Hat, Inc. + * Copyright (C) 2006-2010 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -53,9 +53,9 @@ VIR_ENUM_IMPL(virNetworkForward, VIR_NETWORK_FORWARD_LAST, none, nat, route ) -#define virNetworkReportError(code, fmt...) \ +#define virNetworkReportError(code, ...)\ virReportErrorHelper(NULL, VIR_FROM_NETWORK, code, __FILE__,\ - __FUNCTION__, __LINE__, fmt)
Re: [libvirt] [PATCH] add an assert, to avoid a false-positive NULL-deref warning from clang
According to Jim Meyering on 3/1/2010 10:13 AM: Here's a case in which using an assertion appears to be the only way to tell clang that client really is non-NULL at that point. I'm sure clang's analyzers will eventually improve, and hence avoid this sort of false positive, so have marked this with a FIXME comment, to help ensure we eventually remove this otherwise unnecessary assertion. Thanks for the extra context; it makes in-line review a breeze. @@ -1504,34 +1505,38 @@ static void *qemudWorker(void *data) virMutexLock(server-lock); while (((client = qemudPendingJob(server)) == NULL) !worker-quitRequest) { if (virCondWait(server-job, server-lock) 0) { virMutexUnlock(server-lock); return NULL; } } Indeed, the only way client can be NULL at this point is if worker-quitRequest is true... if (worker-quitRequest) { if (client) virMutexUnlock(client-lock); virMutexUnlock(server-lock); return NULL; } But that means we exit here. worker-processingCall = 1; virMutexUnlock(server-lock); +/* Tell clang we know what we're doing. + FIXME: remove when clang improves. */ +assert (client); So this assertion is valid. ACK, if assert() is okay. On the other hand, perhaps a more invasive rewrite would also work while also avoiding assert(), by hoisting the worker-quitRequest into the while loop, something like: while ((client = qemudPendingJob(server)) == NULL) { if (worker-quitRequest || virCondWait(server-job, server-lock) 0) { virMutexUnlock(server-lock); return NULL; } } if (worker-quitRequest) { virMutexUnlock(client-lock); virMutexUnlock(server-lock); return NULL; } Should I write that into patch format? -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] add an assert, to avoid a false-positive NULL-deref warning from clang
On the other hand, perhaps a more invasive rewrite would also work while also avoiding assert(), by hoisting the worker-quitRequest into the while loop, something like: while ((client = qemudPendingJob(server)) == NULL) { if (worker-quitRequest || virCondWait(server-job,server-lock) 0) { virMutexUnlock(server-lock); return NULL; } } if (worker-quitRequest) { virMutexUnlock(client-lock); virMutexUnlock(server-lock); return NULL; } Should I write that into patch format? I think it's best to report the bug to clang's bugzilla and not clutter libvirt too much. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH] build: consistently use C99 varargs macros
On 03/01/2010 06:24 PM, Daniel P. Berrange wrote: mingw == gcc built as a cross-compiler, so there's no issue there. Unfortunately, _native_ mingw (i.e. building under MSYS) uses a hideously old version of GCC (3.4.x), so it's not always a non-issue. But it is fine in this case: http://gcc.gnu.org/onlinedocs/gcc-3.4.6/cpp/Variadic-Macros.html Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] add an assert, to avoid a false-positive NULL-deref warning from clang
On Mon, Mar 01, 2010 at 10:27:43AM -0700, Eric Blake wrote: According to Jim Meyering on 3/1/2010 10:13 AM: Here's a case in which using an assertion appears to be the only way to tell clang that client really is non-NULL at that point. I'm sure clang's analyzers will eventually improve, and hence avoid this sort of false positive, so have marked this with a FIXME comment, to help ensure we eventually remove this otherwise unnecessary assertion. Thanks for the extra context; it makes in-line review a breeze. @@ -1504,34 +1505,38 @@ static void *qemudWorker(void *data) virMutexLock(server-lock); while (((client = qemudPendingJob(server)) == NULL) !worker-quitRequest) { if (virCondWait(server-job, server-lock) 0) { virMutexUnlock(server-lock); return NULL; } } Indeed, the only way client can be NULL at this point is if worker-quitRequest is true... if (worker-quitRequest) { if (client) virMutexUnlock(client-lock); virMutexUnlock(server-lock); return NULL; } But that means we exit here. worker-processingCall = 1; virMutexUnlock(server-lock); +/* Tell clang we know what we're doing. + FIXME: remove when clang improves. */ +assert (client); So this assertion is valid. ACK, if assert() is okay. In general we really try to avoid assert() basically if we exit that means that the clients loose their access to their VM so that's a really really bad situation that we really try to avoid. Without refactoring assert() in that case would be acceptable because basically that would mean the compiler generated broken code and well, that something we don't try to gard against... On the other hand, perhaps a more invasive rewrite would also work while also avoiding assert(), by hoisting the worker-quitRequest into the while loop, something like: while ((client = qemudPendingJob(server)) == NULL) { if (worker-quitRequest || virCondWait(server-job, server-lock) 0) { virMutexUnlock(server-lock); return NULL; } } if (worker-quitRequest) { virMutexUnlock(client-lock); virMutexUnlock(server-lock); return NULL; } Should I write that into patch format? But if you can rewrite the loops to avoid the assert and keep clang happy, I think it's an even better solution, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/6] Allow a timezone to be specified instead of sync to host timezone
On Mon, Mar 01, 2010 at 03:14:48PM +0100, Daniel Veillard wrote: On Thu, Feb 18, 2010 at 05:54:30PM +, Daniel P. Berrange wrote: This extends the XML to allow for clock offset='timezone' timezone='Europe/Paris'/ This is useful if the admin has not configured any timezone on the host OS, but still wants to synchronize a guest to a specific one. * src/conf/domain_conf.h, src/conf/domain_conf.c: Support extra 'timezone' attribute on clock configuration * docs/schemas/domain.rng: Add 'timezone' attribute * src/xen/xend_internal.c, src/xen/xm_internal.c: Reject configs with a configurable timezone --- docs/schemas/domain.rng | 18 +++--- src/conf/domain_conf.c | 24 src/conf/domain_conf.h | 13 ++--- src/qemu/qemu_conf.c|2 +- src/xen/xend_internal.c | 10 -- src/xen/xm_internal.c | 17 - 6 files changed, 66 insertions(+), 18 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index d295bfe..4a36a97 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -298,9 +298,16 @@ optional element name=clock choice - attribute name=offset -valuelocaltime/value - /attribute + group +attribute name=offset + valuelocaltime/value + /attribute + optional + attribute name=timezone + ref name=timeZone/ + /attribute + /optional + /group attribute name=offset valueutc/value /attribute @@ -1584,4 +1591,9 @@ param name=pattern(-|\+)?[0-9]+/param /data /define + define name=timeZone +data type=string + param name=pattern[a-zA-Z0-9_\.\+\-/]+/param Hum ... I wonder if we should not add ':' as it's used for POSIX TZ How is it used ? I looked at /usr/share/zoneinfo and all the files there should be matched by this regex ok without needing ':' 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 4/6] Allow a timezone to be specified instead of sync to host timezone
According to Daniel P. Berrange on 3/1/2010 11:45 AM: +data type=string + param name=pattern[a-zA-Z0-9_\.\+\-/]+/param Hum ... I wonder if we should not add ':' as it's used for POSIX TZ How is it used ? I looked at /usr/share/zoneinfo and all the files there should be matched by this regex ok without needing ':' POSIX states that the use of ':' introduces implementation-defined extensions. http://www.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html The value of TZ has one of the two forms (spaces inserted for clarity): :characters or: std offset dst offset, rule If TZ is of the first format (that is, if the first character is a colon), the characters following the colon are handled in an implementation-defined manner. In other words, since Europe/Paris is NOT a valid timezone according to POSIX rules, a user concerned about POSIX compliance should be allowed to use TZ=:Europe/Paris to still get glibc semantics of a named zone. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] Add new clock mode allowing variable adjustments
Daniel P. Berrange wrote: Here's the change to the syntax-check rule, followed by the change to the code to make those files comply. If everyone agrees, I'll reverse the order before pushing, so that make syntax-check passes after each commit. ACK, this makes sense - since we clearly keep re-introducing tabs in these files. Thanks for the quick ACK. Pushed. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] x86Decode: avoid NULL-dereference upon questionable input
Clang warned about the potential NULL-dereference via the STREQ/strcmp below. models[i] could be NULL. Even models could be NULL, and the allowed = ... test makes that appear to be deliberately allowed. The change below is the least invasive and cleanest I could come up with, assuming there is no need to diagnose (e.g., by returning -1) the condition of a NULL models[i] pointer. From 623ac546a93f3e5b554647b05524b5041a642530 Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Sun, 28 Feb 2010 13:34:06 +0100 Subject: [PATCH] x86Decode: avoid NULL-dereference upon questionable input * src/cpu/cpu_x86.c (x86Decode): Don't dereference NULL when passed a NULL models pointer, or when passed a nonzero nmodels value and a corresponding NULL models[i]. --- src/cpu/cpu_x86.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 2194c32..b263629 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -957,89 +957,89 @@ x86Compare(virCPUDefPtr host, virCPUDefPtr cpu) { return x86Compute(host, cpu, NULL); } static virCPUCompareResult x86GuestData(virCPUDefPtr host, virCPUDefPtr guest, union cpuData **data) { return x86Compute(host, guest, data); } static int x86Decode(virCPUDefPtr cpu, const union cpuData *data, const char **models, unsigned int nmodels) { int ret = -1; struct x86_map *map; const struct x86_model *candidate; virCPUDefPtr cpuCandidate; virCPUDefPtr cpuModel = NULL; struct cpuX86cpuid *cpuid; int i; if (data == NULL || (map = x86LoadMap()) == NULL) return -1; candidate = map-models; while (candidate != NULL) { bool allowed = (models == NULL); for (i = 0; i candidate-ncpuid; i++) { cpuid = x86DataCpuid(data, candidate-cpuid[i].function); if (cpuid == NULL || !x86cpuidMatchMasked(cpuid, candidate-cpuid + i)) goto next; } for (i = 0; i nmodels; i++) { -if (STREQ(models[i], candidate-name)) { +if (models models[i] STREQ(models[i], candidate-name)) { allowed = true; break; } } if (!allowed) { VIR_DEBUG(CPU model %s not allowed by hypervisor; ignoring, candidate-name); goto next; } if (!(cpuCandidate = x86DataToCPU(data, candidate, map))) goto out; if (cpuModel == NULL || cpuModel-nfeatures cpuCandidate-nfeatures) { virCPUDefFree(cpuModel); cpuModel = cpuCandidate; } else virCPUDefFree(cpuCandidate); next: candidate = candidate-next; } if (cpuModel == NULL) { virCPUReportError(VIR_ERR_INTERNAL_ERROR, %s, _(Cannot find suitable CPU model for given data)); goto out; } cpu-model = cpuModel-model; cpu-nfeatures = cpuModel-nfeatures; cpu-features = cpuModel-features; VIR_FREE(cpuModel); ret = 0; out: x86MapFree(map); virCPUDefFree(cpuModel); return ret; } -- 1.7.0.1.414.g89213d -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] phypUUIDTable_Push: do not corrupt output stream
This started with clang's report of the two dead stores. With the increment and decrement by sent being performed outside of the intended loop, the two assignments had no net effect. BTW, I presume these hard-coded names, from the same function, are somehow temporary and/or debugging-related: char remote_file[] = /home/hscroot/libvirt_uuid_table; char local_file[] = ./uuid_table; From a1104c4d1a95b10cab19bacf91ed073769f1d2d7 Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Mon, 1 Mar 2010 21:26:59 +0100 Subject: [PATCH] phypUUIDTable_Push: do not corrupt output stream upon partial write * src/phyp/phyp_driver.c (phypUUIDTable_Push): Move incr/decr of ptr/nread into the loop where those variables are used. Also, remove exit label and just-preceding goto. --- src/phyp/phyp_driver.c |7 ++- 1 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 1e8ed30..3fc5a0c 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1982,54 +1982,51 @@ phypUUIDTable_Push(virConnectPtr conn) do { nread = fread(buffer, 1, sizeof(buffer), fd); if (nread = 0) { if (feof(fd)) { /* end of file */ break; } else { VIR_ERROR(Failed to read from '%s', local_file); goto err; } } ptr = buffer; sent = 0; do { /* write the same data over and over, until error or completion */ rc = libssh2_channel_write(channel, ptr, nread); if (LIBSSH2_ERROR_EAGAIN == rc) { /* must loop around */ continue; } else if (rc 0) { /* rc indicates how many bytes were written this time */ sent += rc; } +ptr += sent; +nread -= sent; } while (rc 0 sent nread); -ptr += sent; -nread -= sent; } while (1); -goto exit; - - exit: if (channel) { libssh2_channel_send_eof(channel); libssh2_channel_wait_eof(channel); libssh2_channel_wait_closed(channel); libssh2_channel_free(channel); channel = NULL; } return 0; err: if (channel) { libssh2_channel_send_eof(channel); libssh2_channel_wait_eof(channel); libssh2_channel_wait_closed(channel); libssh2_channel_free(channel); channel = NULL; } return -1; } int phypUUIDTable_Pull(virConnectPtr conn) { -- 1.7.0.1.414.g89213d -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] x86Decode: avoid NULL-dereference upon questionable input
According to Jim Meyering on 3/1/2010 1:21 PM: Clang warned about the potential NULL-dereference via the STREQ/strcmp below. models[i] could be NULL. Even models could be NULL, and the allowed = ... test makes that appear to be deliberately allowed. This same function was also listed by coverity, but only for models, not models[i]. The change below is the least invasive and cleanest I could come up with, assuming there is no need to diagnose (e.g., by returning -1) the condition of a NULL models[i] pointer. while (candidate != NULL) { bool allowed = (models == NULL); for (i = 0; i candidate-ncpuid; i++) { cpuid = x86DataCpuid(data, candidate-cpuid[i].function); if (cpuid == NULL || !x86cpuidMatchMasked(cpuid, candidate-cpuid + i)) goto next; } for (i = 0; i nmodels; i++) { -if (STREQ(models[i], candidate-name)) { +if (models models[i] STREQ(models[i], candidate-name)) { Isn't the intent that (models==NULL) iff (nmodels==0)? In which case, this code is unreachable if models is NULL. But your patch certainly is the least-invasive possible, and while it is only a false positive for well-formed arguments, I didn't spend time checking all clients of x86Decode to see if there is ever a possibility of bad arguments. ACK -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] openvzDomainDefineCmd: remove useless increment
According to Jim Meyering on 3/1/2010 1:41 PM: Another dead store. Refreshingly shallow. Subject: [PATCH] openvzDomainDefineCmd: remove useless increment * src/openvz/openvz_driver.c (openvzDomainDefineCmd): Remove useless increment of max_veid. ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] phypUUIDTable_Push: do not corrupt output stream
According to Jim Meyering on 3/1/2010 1:35 PM: This started with clang's report of the two dead stores. With the increment and decrement by sent being performed outside of the intended loop, the two assignments had no net effect. do { /* write the same data over and over, until error or completion */ rc = libssh2_channel_write(channel, ptr, nread); if (LIBSSH2_ERROR_EAGAIN == rc) { /* must loop around */ continue; } else if (rc 0) { /* rc indicates how many bytes were written this time */ sent += rc; } +ptr += sent; +nread -= sent; } while (rc 0 sent nread); -ptr += sent; -nread -= sent; ACK. } while (1); -goto exit; - - exit: Definitely worth the cleanup. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] inability to open local read-only connection
Thanks for your suggestions Daniel! I will rerun my executable thru strace with LIBVIRT_DEBUG=1 to see what happens and will send the results if need be. John -Original Message- From: Daniel P. Berrange [mailto:berra...@redhat.com] Sent: Monday, March 01, 2010 7:58 AM To: Tavares, John Cc: libvir-list@redhat.com Subject: Re: [libvirt] inability to open local read-only connection On Thu, Feb 25, 2010 at 02:34:02PM -0600, Tavares, John wrote: I have been experimenting with using libvirt (0.3.3) on a variety of systems (RHEL, CentOS and Oracle VM). I have run into an issue when I try to open a local read-only connection to the hypervisor that is failing only on Oracle VM server release 2.2.0. I have created a root owned setuid executable that is effectively running as root, but even so, still cannot open the local read-only connection of the hypervisor. It only works if I run it directly as root. This is not an option. I do not understand why it works as is on my RHEL and CentOS machines, but not my Oracle machine. It would seem as thought it is not checking if the effective uid is root, just the uid. A readonly connection essentially just goes to a separate UNIX socket, which has more relaxed permissions (mode 0777, instead of 0700). The kernel does the permissions checking when attempting to open it, so it should be using the effective ID. Has anyone run into a similar issue or have any suggestions of what I might try to fix this issue or can tell me that this is a defect that needs (is) fixed?? I'd suggest trying to strace your process see where it fails, and/or run with LIBVIRT_DEBUG=1 environemnt variable set. 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] [virt-tools-list] and for LXC?
Hello there, libvirt already has a full feature driver for lxc. virt-install/virt-manager still need some love wrt lxc... I've started playing around with that, but that's still only a proof of concept. CR. --- Cleber Rodrigues cr...@redhat.com Solutions Architect - Red Hat, Inc. Mobile: +55 61 9185.3454 - Mensagem original - De: Mihamina Rakotomandimby miham...@gulfsat.mg Para: libvir-list@redhat.com, virt-tools-l...@redhat.com Enviadas: Segunda-feira, 1 de Março de 2010 10:14:34 (GMT-0300) Auto-Detected Assunto: [virt-tools-list] and for LXC? Manao ahoana, Hello, Bonjour, Is there some plan/schedule to make libvirt LXC friendly? Or is it already? Misaotra, Thanks, Merci. -- Architecte Informatique chez Blueline/Gulfsat: Administration Systeme, Recherche Developpement +261 34 29 155 34 / +261 33 11 207 36 ___ virt-tools-list mailing list virt-tools-l...@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Problems with attach-device/detach-device using libvirt 0.7.6
I wrote: [Problems attaching and detaching PCI devices] Ok, today I was working on that again and USB attaching was found to be completely broken, too. Please drop my libvirt-0.7.6-null-pci-id.patch patch from the previous mail and use libvirt-0.7.6-null-device-id.patch instead: USB has the very same problem. The read problem was indeed that attaching USB devices used pci-attach as qemu command instead of usb_add: libvirt-0.7.6-usb-attach.patch comes to help out. And I think it's rather confusing to get messages about errors in PCI device configuration when it's in fact an USB device. This had nothing to do with my problem, but I found that and libvirt-0.7.6-usb-messages.patch fixes it. Eike diff -aurp libvirt-0.7.6-orig/src/qemu/qemu_conf.c libvirt-0.7.6/src/qemu/qemu_conf.c --- libvirt-0.7.6-orig/src/qemu/qemu_conf.c 2010-02-03 17:56:38.0 +0100 +++ libvirt-0.7.6/src/qemu/qemu_conf.c 2010-03-01 16:35:21.0 +0100 @@ -2690,7 +2690,8 @@ qemuBuildPCIHostdevDevStr(virDomainHostd dev-source.subsys.u.pci.bus, dev-source.subsys.u.pci.slot, dev-source.subsys.u.pci.function); -virBufferVSprintf(buf, ,id=%s, dev-info.alias); +if (dev-info.alias != NULL) +virBufferVSprintf(buf, ,id=%s, dev-info.alias); if (qemuBuildDeviceAddressStr(buf, dev-info) 0) goto error; @@ -2734,11 +2735,12 @@ qemuBuildUSBHostdevDevStr(virDomainHostd return NULL; } -if (virAsprintf(ret, usb-host,hostbus=%.3d,hostaddr=%.3d,id=%s, +if (virAsprintf(ret, usb-host,hostbus=%.3d,hostaddr=%.3d, dev-source.subsys.u.usb.bus, -dev-source.subsys.u.usb.device, -dev-info.alias) 0) +dev-source.subsys.u.usb.device) 0) virReportOOMError(NULL); +if (dev-info.alias != NULL) +virBufferVSprintf(ret, ,id=%s, dev-info.alias); return ret; } diff -aurp libvirt-0.7.6-orig/src/qemu/qemu_driver.c libvirt-0.7.6/src/qemu/qemu_driver.c --- libvirt-0.7.6-orig/src/qemu/qemu_driver.c 2010-02-03 16:56:00.0 +0100 +++ libvirt-0.7.6/src/qemu/qemu_driver.c 2010-03-01 16:32:26.0 +0100 @@ -5831,7 +5831,7 @@ static int qemudDomainAttachHostUsbDevic char *devstr = NULL; if ((qemuCmdFlags QEMUD_CMD_FLAG_DEVICE) -!(devstr = qemuBuildPCIHostdevDevStr(hostdev))) +!(devstr = qemuBuildUSBHostdevDevStr(hostdev))) goto error; if (VIR_REALLOC_N(vm-def-hostdevs, vm-def-nhostdevs+1) 0) { diff -aurp libvirt-0.7.6-orig/src/qemu/qemu_conf.c libvirt-0.7.6/src/qemu/qemu_conf.c --- libvirt-0.7.6-orig/src/qemu/qemu_conf.c 2010-02-03 17:56:38.0 +0100 +++ libvirt-0.7.6/src/qemu/qemu_conf.c 2010-03-01 16:35:21.0 +0100 @@ -4776,7 +4776,7 @@ qemuParseCommandLineUSB(virConnectPtr co if (!STRPREFIX(val, host:)) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _(unknown PCI device syntax '%s'), val); + _(unknown USB device syntax '%s'), val); VIR_FREE(def); goto cleanup; } @@ -4792,21 +4792,21 @@ qemuParseCommandLineUSB(virConnectPtr co start = end + 1; if (virStrToLong_i(start, NULL, 16, second) 0) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _(cannot extract PCI device product '%s'), val); + _(cannot extract USB device product '%s'), val); VIR_FREE(def); goto cleanup; } } else { if (virStrToLong_i(start, end, 10, first) 0 || !end || *end != '.') { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _(cannot extract PCI device bus '%s'), val); + _(cannot extract USB device bus '%s'), val); VIR_FREE(def); goto cleanup; } start = end + 1; if (virStrToLong_i(start, NULL, 10, second) 0) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _(cannot extract PCI device address '%s'), val); + _(cannot extract USB device address '%s'), val); VIR_FREE(def); goto cleanup; } signature.asc Description: This is a digitally signed message part. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] running libvirt on Xen Cloud Platform 0.7.1
Hi, I am trying to run libvirt daemon in Dom0 of Xen Cloud Platform 0.7.1. For that purpose I downloaded the libvirt source files 0.7.6 and trying to build the libvirt rpm with XEN driver using the XCP DDK. I am running into to many problems getting it done - so I was wondering whether this is supported or if anyone is using it in such way. Thank You, Gadi _ Hotmail: Free, trusted and rich email service. https://signup.live.com/signup.aspx?id=60969-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Problems with attach-device/detach-device using libvirt 0.7.6
On Mon, Mar 01, 2010 at 07:47:20PM +0100, Rolf Eike Beer wrote: I wrote: [Problems attaching and detaching PCI devices] Ok, today I was working on that again and USB attaching was found to be completely broken, too. Please drop my libvirt-0.7.6-null-pci-id.patch patch from the previous mail and use libvirt-0.7.6-null-device-id.patch instead: USB has the very same problem. The read problem was indeed that attaching USB devices used pci-attach as qemu command instead of usb_add: libvirt-0.7.6-usb-attach.patch comes to help out. And I think it's rather confusing to get messages about errors in PCI device configuration when it's in fact an USB device. This had nothing to do with my problem, but I found that and libvirt-0.7.6-usb-messages.patch fixes it. Eike diff -aurp libvirt-0.7.6-orig/src/qemu/qemu_conf.c libvirt-0.7.6/src/qemu/qemu_conf.c --- libvirt-0.7.6-orig/src/qemu/qemu_conf.c 2010-02-03 17:56:38.0 +0100 +++ libvirt-0.7.6/src/qemu/qemu_conf.c2010-03-01 16:35:21.0 +0100 @@ -2690,7 +2690,8 @@ qemuBuildPCIHostdevDevStr(virDomainHostd dev-source.subsys.u.pci.bus, dev-source.subsys.u.pci.slot, dev-source.subsys.u.pci.function); -virBufferVSprintf(buf, ,id=%s, dev-info.alias); +if (dev-info.alias != NULL) +virBufferVSprintf(buf, ,id=%s, dev-info.alias); if (qemuBuildDeviceAddressStr(buf, dev-info) 0) goto error; @@ -2734,11 +2735,12 @@ qemuBuildUSBHostdevDevStr(virDomainHostd return NULL; } -if (virAsprintf(ret, usb-host,hostbus=%.3d,hostaddr=%.3d,id=%s, +if (virAsprintf(ret, usb-host,hostbus=%.3d,hostaddr=%.3d, dev-source.subsys.u.usb.bus, -dev-source.subsys.u.usb.device, -dev-info.alias) 0) +dev-source.subsys.u.usb.device) 0) virReportOOMError(NULL); +if (dev-info.alias != NULL) +virBufferVSprintf(ret, ,id=%s, dev-info.alias); return ret; } This patch isn't correct - all devices *must* have a alias assigned before hotplug. The real issue here is the hotplug method is missing the API call to assign the alias. diff -aurp libvirt-0.7.6-orig/src/qemu/qemu_driver.c libvirt-0.7.6/src/qemu/qemu_driver.c --- libvirt-0.7.6-orig/src/qemu/qemu_driver.c 2010-02-03 16:56:00.0 +0100 +++ libvirt-0.7.6/src/qemu/qemu_driver.c 2010-03-01 16:32:26.0 +0100 @@ -5831,7 +5831,7 @@ static int qemudDomainAttachHostUsbDevic char *devstr = NULL; if ((qemuCmdFlags QEMUD_CMD_FLAG_DEVICE) -!(devstr = qemuBuildPCIHostdevDevStr(hostdev))) +!(devstr = qemuBuildUSBHostdevDevStr(hostdev))) goto error; if (VIR_REALLOC_N(vm-def-hostdevs, vm-def-nhostdevs+1) 0) { diff -aurp libvirt-0.7.6-orig/src/qemu/qemu_conf.c libvirt-0.7.6/src/qemu/qemu_conf.c --- libvirt-0.7.6-orig/src/qemu/qemu_conf.c 2010-02-03 17:56:38.0 +0100 +++ libvirt-0.7.6/src/qemu/qemu_conf.c2010-03-01 16:35:21.0 +0100 @@ -4776,7 +4776,7 @@ qemuParseCommandLineUSB(virConnectPtr co if (!STRPREFIX(val, host:)) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _(unknown PCI device syntax '%s'), val); + _(unknown USB device syntax '%s'), val); VIR_FREE(def); goto cleanup; } @@ -4792,21 +4792,21 @@ qemuParseCommandLineUSB(virConnectPtr co start = end + 1; if (virStrToLong_i(start, NULL, 16, second) 0) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _(cannot extract PCI device product '%s'), val); + _(cannot extract USB device product '%s'), val); VIR_FREE(def); goto cleanup; } } else { if (virStrToLong_i(start, end, 10, first) 0 || !end || *end != '.') { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _(cannot extract PCI device bus '%s'), val); + _(cannot extract USB device bus '%s'), val); VIR_FREE(def); goto cleanup; } start = end + 1; if (virStrToLong_i(start, NULL, 10, second) 0) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _(cannot extract PCI device address '%s'), val); + _(cannot extract USB device address '%s'), val); VIR_FREE(def); goto cleanup; } These two look good i'll apply them 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
Re: [libvirt] [PATCH] x86Decode: avoid NULL-dereference upon questionable input
Eric Blake wrote: According to Jim Meyering on 3/1/2010 1:21 PM: Clang warned about the potential NULL-dereference via the STREQ/strcmp below. models[i] could be NULL. Even models could be NULL, and the allowed = ... test makes that appear to be deliberately allowed. This same function was also listed by coverity, but only for models, not models[i]. Yes, I was disappointed to see Coverity missed that. The change below is the least invasive and cleanest I could come up with, assuming there is no need to diagnose (e.g., by returning -1) the condition of a NULL models[i] pointer. while (candidate != NULL) { bool allowed = (models == NULL); for (i = 0; i candidate-ncpuid; i++) { cpuid = x86DataCpuid(data, candidate-cpuid[i].function); if (cpuid == NULL || !x86cpuidMatchMasked(cpuid, candidate-cpuid + i)) goto next; } for (i = 0; i nmodels; i++) { -if (STREQ(models[i], candidate-name)) { +if (models models[i] STREQ(models[i], candidate-name)) { Isn't the intent that (models==NULL) iff (nmodels==0)? That is the intent, but the code at this level does not detect the mismatch. I think someone made a change recently to protect us at a higher (cpu-independent) level. But that doesn't help us here, if a new caller of this function violates those higher-level constraints. In which case, this code is unreachable if models is NULL. But your patch certainly is the least-invasive possible, and while it is only a false positive for well-formed arguments, I didn't spend time checking all clients of x86Decode to see if there is ever a possibility of bad arguments. ACK Thanks. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix USB/PCI device address aliases in QEMU hotplug driver
The USB/PCI device hotplug code for the QEMU driver was forgetting to allocate a unique device alias. * src/qemu/qemu_driver.c: Fill in device alias for USB/PCI devices --- src/qemu/qemu_driver.c | 11 --- 1 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c2bd46d..5ac3316 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5878,6 +5878,8 @@ static int qemudDomainAttachHostPciDevice(struct qemud_driver *driver, } if (qemuCmdFlags QEMUD_CMD_FLAG_DEVICE) { +if (qemuAssignDeviceHostdevAlias(vm-def, hostdev, -1) 0) +goto error; if (qemuDomainPCIAddressEnsureAddr(priv-pciaddrs, hostdev-info) 0) goto error; @@ -5926,9 +5928,12 @@ static int qemudDomainAttachHostUsbDevice(struct qemud_driver *driver, qemuDomainObjPrivatePtr priv = vm-privateData; char *devstr = NULL; -if ((qemuCmdFlags QEMUD_CMD_FLAG_DEVICE) -!(devstr = qemuBuildUSBHostdevDevStr(hostdev))) -goto error; +if (qemuCmdFlags QEMUD_CMD_FLAG_DEVICE) { +if (qemuAssignDeviceHostdevAlias(vm-def, hostdev, -1) 0) +goto error; +if (!(devstr = qemuBuildUSBHostdevDevStr(hostdev))) +goto error; +} if (VIR_REALLOC_N(vm-def-hostdevs, vm-def-nhostdevs+1) 0) { virReportOOMError(); -- 1.6.6.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/6] Allow a timezone to be specified instead of sync to host timezone
On Mon, Mar 01, 2010 at 06:45:28PM +, Daniel P. Berrange wrote: On Mon, Mar 01, 2010 at 03:14:48PM +0100, Daniel Veillard wrote: On Thu, Feb 18, 2010 at 05:54:30PM +, Daniel P. Berrange wrote: This extends the XML to allow for clock offset='timezone' timezone='Europe/Paris'/ This is useful if the admin has not configured any timezone on the host OS, but still wants to synchronize a guest to a specific one. * src/conf/domain_conf.h, src/conf/domain_conf.c: Support extra 'timezone' attribute on clock configuration * docs/schemas/domain.rng: Add 'timezone' attribute * src/xen/xend_internal.c, src/xen/xm_internal.c: Reject configs with a configurable timezone --- docs/schemas/domain.rng | 18 +++--- src/conf/domain_conf.c | 24 src/conf/domain_conf.h | 13 ++--- src/qemu/qemu_conf.c|2 +- src/xen/xend_internal.c | 10 -- src/xen/xm_internal.c | 17 - 6 files changed, 66 insertions(+), 18 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index d295bfe..4a36a97 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -298,9 +298,16 @@ optional element name=clock choice - attribute name=offset -valuelocaltime/value - /attribute + group +attribute name=offset + valuelocaltime/value + /attribute + optional + attribute name=timezone + ref name=timeZone/ + /attribute + /optional + /group attribute name=offset valueutc/value /attribute @@ -1584,4 +1591,9 @@ param name=pattern(-|\+)?[0-9]+/param /data /define + define name=timeZone +data type=string + param name=pattern[a-zA-Z0-9_\.\+\-/]+/param Hum ... I wonder if we should not add ':' as it's used for POSIX TZ How is it used ? I looked at /usr/share/zoneinfo and all the files there should be matched by this regex ok without needing ':' I looked there http://www.gnu.org/s/libc/manual/html_node/TZ-Variable.html seems that the offset allows for : separator for minutes/seconds I'm sure there is at least some places not aligned on hourly boundaries :) But honnestly it's not a big deal ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt/dnsmasq integration.
Daniel P. Berrange wrote: The total set of DNSMASQ args that we currently use are --strict-order --bind-interfaces --domain DOMAIN-NAME(optional) --pid-file=/var/run/libvirt/network/$NETWORK.pid --conf-file= --listen-address=IPADDR-OF-BRIDGE --except-interface=lo --dhcp-range=IPRANGE(optional, multiple times) --dhcp-lease-max=RANGE-SIZE (optional) --dhcp-host=STATIC-HOST-MAPPING (optional) --enable-tftp (optional) --tftp-root=/some/path (optional) --dhcp-boot=PXE-BOOT-SERVER (optional) OK, the use of TFTP by libvirt is new to me. Looks like I'll have to pull some similar tricks to allow libvirt to enable TFTP on on interface without disturbing things elsewhere. We only very recently added the TFTP/boot options so it is probably not visible in many Linux distros yet. So, if we solve the extra TFTP interface binding issue, we shouldn't need to worry about a formal 'scope', so lets ignore that suggestion of mine. I propose that your dhcp-range should become dhcp-range=interface:virt0, set:virt0tag, IPRANGE dhcp-boot=tag:virt0tag,PXE_BOOT_SERVER That engages the isolation mode for DHCP and ensures that the correct PXE server is supplied to your clients even if a different one is configured elsewhere. (The set: and tag: syntax is just new sugar around the existing and rather confusing tagging code) TFTP becomes. enable-tftp=virt0 for a similar isolation mode, and tftp-root=/some/path,virt0 to have a root path which overrides one specified elsewhere. Some preliminary code which implements the facilities required is at http://www.thekelleys.org.uk/dnsmasq/test-releases/dnsmasq-2.53test12.tar.gz Cheers, Simon. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/5] build: use C99 varargs
Here's the full cleanup, based on my earlier RFC. I've tested that everything still builds and passes 'make syntax-check'. This is broken out in chunks, although whoever ends up pushing may want to squash it all into one. src/conf/cpu_conf.c|4 ++-- src/conf/domain_conf.c |4 ++-- src/conf/domain_event.c|5 +++-- src/conf/interface_conf.c |4 ++-- src/conf/network_conf.c|6 +++--- src/conf/node_device_conf.h|5 +++-- src/conf/secret_conf.h |6 +++--- src/conf/storage_conf.h|6 +++--- src/cpu/cpu.h |6 +++--- src/datatypes.c|6 +++--- src/esx/esx_device_monitor.c |5 +++-- src/esx/esx_driver.c |5 +++-- src/esx/esx_interface_driver.c |5 +++-- src/esx/esx_network_driver.c |5 +++-- src/esx/esx_secret_driver.c|5 +++-- src/esx/esx_storage_driver.c |5 +++-- src/esx/esx_util.c |5 +++-- src/esx/esx_vi.c |5 +++-- src/esx/esx_vi_methods.c |5 +++-- src/esx/esx_vi_types.c |5 +++-- src/esx/esx_vmx.c |5 +++-- src/interface/netcf_driver.c |6 +++--- src/libvirt.c |6 +++--- src/lxc/lxc_conf.h |5 +++-- src/network/bridge_driver.c|4 ++-- src/nodeinfo.c |6 +++--- src/opennebula/one_conf.h |8 +--- src/openvz/openvz_conf.h |5 +++-- src/phyp/phyp_driver.c |5 +++-- src/qemu/qemu_conf.h |4 ++-- src/remote/remote_driver.c |4 ++-- src/security/security_driver.h |6 +++--- src/test/test_driver.c |4 ++-- src/uml/uml_conf.h |6 +++--- src/util/hostusb.c |4 ++-- src/util/json.c|6 +++--- src/util/macvtap.c |5 +++-- src/util/pci.c |4 ++-- src/util/stats_linux.c |6 +++--- src/util/util.c|4 ++-- src/util/xml.c |6 +++--- src/vbox/vbox_driver.c |5 +++-- src/vbox/vbox_tmpl.c |5 +++-- src/xen/proxy_internal.c |4 ++-- src/xen/sexpr.c|9 - src/xen/xen_driver.c |4 ++-- src/xen/xen_hypervisor.c |6 +++--- src/xen/xen_inotify.c |5 +++-- src/xen/xend_internal.c|9 - src/xen/xm_internal.c |6 +++--- src/xen/xs_internal.c |4 ++-- 51 files changed, 144 insertions(+), 124 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/5] build: consistently use C99 varargs macros in xen
Continuation of previous patch. * src/xen/proxy_internal.c (virProxyError): Use C99 rather than GNU vararg macro syntax. * src/xen/sexpr.c (virSexprError): Likewise. * src/xen/xen_driver.c (xenUnifiedError): Likewise. * src/xen/xen_hypervisor.c (virXenError): Likewise. * src/xen/xen_inotify.c (virXenInotifyError): Likewise. * src/xen/xend_internal.c (virXendError): Likewise. * src/xen/xm_internal.c (xenXMError): Likewise. * src/xen/xs_internal.c (virXenStoreError): Likewise. --- src/xen/proxy_internal.c |4 ++-- src/xen/sexpr.c |9 - src/xen/xen_driver.c |4 ++-- src/xen/xen_hypervisor.c |6 +++--- src/xen/xen_inotify.c|5 +++-- src/xen/xend_internal.c |9 - src/xen/xm_internal.c|6 +++--- src/xen/xs_internal.c|4 ++-- 8 files changed, 23 insertions(+), 24 deletions(-) diff --git a/src/xen/proxy_internal.c b/src/xen/proxy_internal.c index 8e1c226..109b99b 100644 --- a/src/xen/proxy_internal.c +++ b/src/xen/proxy_internal.c @@ -92,9 +92,9 @@ struct xenUnifiedDriver xenProxyDriver = { * * / -#define virProxyError(conn, code, fmt...)\ +#define virProxyError(conn, code, ...) \ virReportErrorHelper(conn, VIR_FROM_PROXY, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) / * * diff --git a/src/xen/sexpr.c b/src/xen/sexpr.c index 840d3e5..7e370db 100644 --- a/src/xen/sexpr.c +++ b/src/xen/sexpr.c @@ -1,9 +1,8 @@ /* * sexpr.c : S-Expression routines to communicate with the Xen Daemon * - * Copyright (C) 2005 - * - * Anthony Liguori aligu...@us.ibm.com + * Copyright (C) 2010 Red Hat, Inc. + * Copyright (C) 2005 Anthony Liguori aligu...@us.ibm.com * * This file is subject to the terms and conditions of the GNU Lesser General * Public License. See the file COPYING.LIB in the main directory of this @@ -25,9 +24,9 @@ #define VIR_FROM_THIS VIR_FROM_SEXPR -#define virSexprError(code, fmt...) \ +#define virSexprError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_SEXPR, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) /** * sexpr_new: diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 700682c..eac485f 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -72,9 +72,9 @@ static struct xenUnifiedDriver const * const drivers[XEN_UNIFIED_NR_DRIVERS] = { static int inside_daemon; -#define xenUnifiedError(conn, code, fmt...) \ +#define xenUnifiedError(conn, code, ...) \ virReportErrorHelper(conn, VIR_FROM_XEN, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) /** * xenNumaInit: diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index ab8fbc3..fe49ac2 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -803,10 +803,10 @@ struct xenUnifiedDriver xenHypervisorDriver = { }; #endif /* !PROXY */ -#define virXenError(conn, code, fmt...) \ -if (in_init == 0)\ +#define virXenError(conn, code, ...) \ +if (in_init == 0) \ virReportErrorHelper(conn, VIR_FROM_XEN, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) #ifndef PROXY diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c index de91dc6..ee24bc4 100644 --- a/src/xen/xen_inotify.c +++ b/src/xen/xen_inotify.c @@ -4,6 +4,7 @@ */etc/xen */var/lib/xend/domains * + * Copyright (C) 2010 Red Hat, Inc. * Copyright (C) 2008 VirtualIron * * This library is free software; you can redistribute it and/or @@ -43,9 +44,9 @@ #define VIR_FROM_THIS VIR_FROM_XEN_INOTIFY -#define virXenInotifyError(conn, code, fmt...) \ +#define virXenInotifyError(conn, code, ...) \ virReportErrorHelper(NULL, VIR_FROM_XEN_INOTIFY, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__,
[libvirt] [PATCH 1/5] build: consistently use C99 varargs macros in conf
Prior to this patch, there was an inconsistent mix between GNU: and C99: For consistency, and potential portability to other compilers, stick with the C99 vararg macro syntax. * src/conf/cpu_conf.c (virCPUReportError): Use C99 rather than GNU vararg macro syntax. * src/conf/domain_conf.c (virDomainReportError): Likewise. * src/conf/domain_event.c (eventReportError): Likewise. * src/conf/interface_conf.c (virInterfaceReportError): Likewise. * src/conf/network_conf.c (virNetworkReportError): Likewise. * src/conf/node_device_conf.h (virNodeDeviceReportError): Likewise. * src/conf/secret_conf.h (virSecretReportError): Likewise. * src/conf/storage_conf.h (virStorageReportError): Likewise. --- src/conf/cpu_conf.c |4 ++-- src/conf/domain_conf.c |4 ++-- src/conf/domain_event.c |5 +++-- src/conf/interface_conf.c |4 ++-- src/conf/network_conf.c |6 +++--- src/conf/node_device_conf.h |5 +++-- src/conf/secret_conf.h |6 +++--- src/conf/storage_conf.h |6 +++--- 8 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index ed83188..612e376 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -31,9 +31,9 @@ #define VIR_FROM_THIS VIR_FROM_CPU -#define virCPUReportError(code, fmt...) \ +#define virCPUReportError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_CPU, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) VIR_ENUM_IMPL(virCPUMatch, VIR_CPU_MATCH_LAST, minimum, diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index df1ec18..96ba0b0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -238,9 +238,9 @@ VIR_ENUM_IMPL(virDomainClockOffset, VIR_DOMAIN_CLOCK_OFFSET_LAST, variable, timezone); -#define virDomainReportError(code, fmt...) \ +#define virDomainReportError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_DOMAIN, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) #ifndef PROXY diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index b474b1c..e791cab 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -1,6 +1,7 @@ /* * domain_event.c: domain event queue processing helpers * + * Copyright (C) 2010 Red Hat, Inc. * Copyright (C) 2008 VirtualIron * * This library is free software; you can redistribute it and/or @@ -30,9 +31,9 @@ #define VIR_FROM_THIS VIR_FROM_NONE -#define eventReportError(conn, code, fmt...)\ +#define eventReportError(conn, code, ...) \ virReportErrorHelper(conn, VIR_FROM_THIS, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) /** diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index a0d2dfa..bc9e958 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -45,9 +45,9 @@ static int virInterfaceDefDevFormat(virBufferPtr buf, const virInterfaceDefPtr def, int level); -#define virInterfaceReportError(code, fmt...) \ +#define virInterfaceReportError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_INTERFACE, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) static void virInterfaceIpDefFree(virInterfaceIpDefPtr def) { diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 6d3c3c0..39ebd62 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1,7 +1,7 @@ /* * network_conf.c: network XML handling * - * Copyright (C) 2006-2009 Red Hat, Inc. + * Copyright (C) 2006-2010 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -53,9 +53,9 @@ VIR_ENUM_IMPL(virNetworkForward, VIR_NETWORK_FORWARD_LAST, none, nat, route ) -#define virNetworkReportError(code, fmt...) \ +#define virNetworkReportError(code, ...)\ virReportErrorHelper(NULL, VIR_FROM_NETWORK, code, __FILE__,\ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) virNetworkObjPtr virNetworkFindByUUID(const virNetworkObjListPtr nets, const unsigned char *uuid) diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 3a5432c..cbaad9b 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h
[libvirt] [PATCH 5/5] build: consistently use C99 varargs macros throughout
Conclusion of previous patches. * src/cpu/cpu.h (virCPUReportError): Use C99 rather than GNU vararg macro syntax. * src/datatypes.c (virLibConnError): Likewise. * src/interface/netcf_driver.c (interfaceReportError): Likewise. * src/libvirt.c (virLibStreamError): Likewise. * src/lxc/lxc_conf.h (lxcError): Likewise. * src/network/bridge_driver.c (networkReportError): Likewise. * src/nodeinfo.c (nodeReportError): Likewise. * src/opennebula/one_conf.h (oneError): Likewise. * src/openvz/openvz_conf.h (openvzError): Likewise. * src/phyp/phyp_driver.c (PHYP_ERROR): Likewise. * src/qemu/qemu_conf.h (qemuReportError): Likewise. * src/remote/remote_driver.c (errorf): Likewise. * src/security/security_driver.h (virSecurityReportError): Likewise. * src/test/test_driver.c (testError): Likewise. * src/uml/uml_conf.h (umlReportError): Likewise. * src/vbox/vbox_driver.c (vboxError): Likewise. * src/vbox/vbox_tmpl.c (vboxError): Likewise. --- src/cpu/cpu.h |6 +++--- src/datatypes.c|6 +++--- src/interface/netcf_driver.c |6 +++--- src/libvirt.c |6 +++--- src/lxc/lxc_conf.h |5 +++-- src/network/bridge_driver.c|4 ++-- src/nodeinfo.c |6 +++--- src/opennebula/one_conf.h |8 +--- src/openvz/openvz_conf.h |5 +++-- src/phyp/phyp_driver.c |5 +++-- src/qemu/qemu_conf.h |4 ++-- src/remote/remote_driver.c |4 ++-- src/security/security_driver.h |6 +++--- src/test/test_driver.c |4 ++-- src/uml/uml_conf.h |6 +++--- src/vbox/vbox_driver.c |5 +++-- src/vbox/vbox_tmpl.c |5 +++-- 17 files changed, 49 insertions(+), 42 deletions(-) diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index c8d961d..4287ca3 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -1,7 +1,7 @@ /* * cpu.h: internal functions for CPU manipulation * - * Copyright (C) 2009--2010 Red Hat, Inc. + * Copyright (C) 2009-2010 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -30,9 +30,9 @@ #include cpu_x86_data.h -#define virCPUReportError(code, fmt...) \ +#define virCPUReportError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_CPU, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) union cpuData { diff --git a/src/datatypes.c b/src/datatypes.c index fa342e0..01601d8 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -1,7 +1,7 @@ /* * datatypes.h: management of structs for public data types * - * Copyright (C) 2006-2009 Red Hat, Inc. + * Copyright (C) 2006-2010 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -31,9 +31,9 @@ #define VIR_FROM_THIS VIR_FROM_NONE -#define virLibConnError(conn, code, fmt...) \ +#define virLibConnError(conn, code, ...) \ virReportErrorHelper(conn, VIR_FROM_THIS, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) / * * diff --git a/src/interface/netcf_driver.c b/src/interface/netcf_driver.c index 7f4d43d..e44156e 100644 --- a/src/interface/netcf_driver.c +++ b/src/interface/netcf_driver.c @@ -2,7 +2,7 @@ * interface_driver.c: backend driver methods to handle physical * interface configuration using the netcf library. * - * Copyright (C) 2006-2009 Red Hat, Inc. + * Copyright (C) 2006-2010 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -33,9 +33,9 @@ #define VIR_FROM_THIS VIR_FROM_INTERFACE -#define interfaceReportError(conn, dom, net, code, fmt...)\ +#define interfaceReportError(conn, dom, net, code, ...) \ virReportErrorHelper(conn, VIR_FROM_THIS, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) /* Main driver state */ struct interface_driver diff --git a/src/libvirt.c b/src/libvirt.c index 9d50c92..b664728 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2,7 +2,7 @@ * libvirt.c: Main interfaces for the libvirt library to handle virtualization * domains from a process running in domain 0 * - * Copyright (C) 2005,2006,2008,2009 Red Hat, Inc. + * Copyright (C) 2005-2006, 2008-2010 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -618,9 +618,9 @@
[libvirt] [PATCH 2/5] build: consistently use C99 varargs macros in esx
Continuation of previous patch. * src/esx/esx_device_monitor.c (ESX_ERROR): Use C99 rather than GNU vararg macro syntax. * src/esx/esx_driver.c (ESX_ERROR): Likewise. * src/esx/esx_interface_driver.c (ESX_ERROR): Likewise. * src/esx/esx_network_driver.c (ESX_ERROR): Likewise. * src/esx/esx_secret_driver.c (ESX_ERROR): Likewise. * src/esx/esx_storage_driver.c (ESX_ERROR): Likewise. * src/esx/esx_util.c (ESX_ERROR): Likewise. * src/esx/esx_vi.c (ESX_VI_ERROR): Likewise. * src/esx/esx_vi_methods.c (ESX_VI_ERROR): Likewise. * src/esx/esx_vi_types.c (ESX_VI_ERROR): Likewise. * src/esx/esx_vmx.c (ESX_ERROR): Likewise. --- src/esx/esx_device_monitor.c |5 +++-- src/esx/esx_driver.c |5 +++-- src/esx/esx_interface_driver.c |5 +++-- src/esx/esx_network_driver.c |5 +++-- src/esx/esx_secret_driver.c|5 +++-- src/esx/esx_storage_driver.c |5 +++-- src/esx/esx_util.c |5 +++-- src/esx/esx_vi.c |5 +++-- src/esx/esx_vi_methods.c |5 +++-- src/esx/esx_vi_types.c |5 +++-- src/esx/esx_vmx.c |5 +++-- 11 files changed, 33 insertions(+), 22 deletions(-) diff --git a/src/esx/esx_device_monitor.c b/src/esx/esx_device_monitor.c index de67ebb..78de0e0 100644 --- a/src/esx/esx_device_monitor.c +++ b/src/esx/esx_device_monitor.c @@ -3,6 +3,7 @@ * esx_device_monitor.c: device monitor methods for managing VMware ESX * host devices * + * Copyright (C) 2010 Red Hat, Inc. * Copyright (C) 2010 Matthias Bolte matthias.bo...@googlemail.com * * This library is free software; you can redistribute it and/or @@ -37,9 +38,9 @@ #define VIR_FROM_THIS VIR_FROM_ESX -#define ESX_ERROR(conn, code, fmt...) \ +#define ESX_ERROR(conn, code, ...)\ virReportErrorHelper(conn, VIR_FROM_ESX, code, __FILE__, __FUNCTION__,\ - __LINE__, fmt) + __LINE__, __VA_ARGS__) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index e125a09..2b52884 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -2,6 +2,7 @@ /* * esx_driver.c: core driver methods for managing VMware ESX hosts * + * Copyright (C) 2010 Red Hat, Inc. * Copyright (C) 2009, 2010 Matthias Bolte matthias.bo...@googlemail.com * Copyright (C) 2009 Maximilian Wilhelm m...@rfc2324.org * @@ -46,9 +47,9 @@ #define VIR_FROM_THIS VIR_FROM_ESX -#define ESX_ERROR(code, fmt...) \ +#define ESX_ERROR(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_ESX, code, __FILE__, __FUNCTION__,\ - __LINE__, fmt) + __LINE__, __VA_ARGS__) static int esxDomainGetMaxVcpus(virDomainPtr domain); diff --git a/src/esx/esx_interface_driver.c b/src/esx/esx_interface_driver.c index 6e9d224..e3739f7 100644 --- a/src/esx/esx_interface_driver.c +++ b/src/esx/esx_interface_driver.c @@ -3,6 +3,7 @@ * esx_interface_driver.h: interface driver methods for managing VMware ESX * host interfaces * + * Copyright (C) 2010 Red Hat, Inc. * Copyright (C) 2010 Matthias Bolte matthias.bo...@googlemail.com * * This library is free software; you can redistribute it and/or @@ -37,9 +38,9 @@ #define VIR_FROM_THIS VIR_FROM_ESX -#define ESX_ERROR(conn, code, fmt...) \ +#define ESX_ERROR(conn, code, ...)\ virReportErrorHelper(conn, VIR_FROM_ESX, code, __FILE__, __FUNCTION__,\ - __LINE__, fmt) + __LINE__, __VA_ARGS__) diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c index 1eeae44..dacc95b 100644 --- a/src/esx/esx_network_driver.c +++ b/src/esx/esx_network_driver.c @@ -3,6 +3,7 @@ * esx_network_driver.c: network driver methods for managing VMware ESX * host networks * + * Copyright (C) 2010 Red Hat, Inc. * Copyright (C) 2010 Matthias Bolte matthias.bo...@googlemail.com * * This library is free software; you can redistribute it and/or @@ -37,9 +38,9 @@ #define VIR_FROM_THIS VIR_FROM_ESX -#define ESX_ERROR(conn, code, fmt...) \ +#define ESX_ERROR(conn, code, ...)\ virReportErrorHelper(conn, VIR_FROM_ESX, code, __FILE__, __FUNCTION__,\ - __LINE__, fmt) + __LINE__, __VA_ARGS__) diff --git a/src/esx/esx_secret_driver.c b/src/esx/esx_secret_driver.c index 7b24d0c..07a178f 100644 --- a/src/esx/esx_secret_driver.c +++ b/src/esx/esx_secret_driver.c @@ -2,6 +2,7 @@ /* * esx_secret_driver.c: secret driver methods for VMware ESX secret manipulation * + * Copyright (C) 2010 Red Hat, Inc. * Copyright (C) 2010
[libvirt] [PATCH 3/5] build: consistently use C99 varargs macros in util
Continuation of previous patch. * src/util/hostusb.c (usbReportError): Use C99 rather than GNU vararg macro syntax. * src/util/json.c (virJSONError): Likewise. * src/util/macvtap.c (ReportError): Likewise. * src/util/pci.c (pciReportError): Likewise. * src/util/stats_linux.c (virStatsError): Likewise. * src/util/util.c (virUtilError): Likewise. * src/util/xml.c (virXMLError): Likewise. --- src/util/hostusb.c |4 ++-- src/util/json.c|6 +++--- src/util/macvtap.c |5 +++-- src/util/pci.c |4 ++-- src/util/stats_linux.c |6 +++--- src/util/util.c|4 ++-- src/util/xml.c |6 +++--- 7 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/util/hostusb.c b/src/util/hostusb.c index bf96539..4f57b59 100644 --- a/src/util/hostusb.c +++ b/src/util/hostusb.c @@ -54,9 +54,9 @@ struct _usbDevice { /* For virReportOOMError() and virReportSystemError() */ #define VIR_FROM_THIS VIR_FROM_NONE -#define usbReportError(code, fmt...) \ +#define usbReportError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_NONE, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) static int usbSysReadFile(const char *f_name, const char *d_name, int base, unsigned *value) diff --git a/src/util/json.c b/src/util/json.c index 3abff58..e388287 100644 --- a/src/util/json.c +++ b/src/util/json.c @@ -1,8 +1,8 @@ /* * json.c: JSON object parsing/formatting * - * Copyright (C) 2009 Daniel P. Berrange * Copyright (C) 2009-2010 Red Hat, Inc. + * Copyright (C) 2009 Daniel P. Berrange * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -36,9 +36,9 @@ /* XXX fixme */ #define VIR_FROM_THIS VIR_FROM_NONE -#define virJSONError(code, fmt...) \ +#define virJSONError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_NONE, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) typedef struct _virJSONParserState virJSONParserState; diff --git a/src/util/macvtap.c b/src/util/macvtap.c index 8c29588..3b526ce 100644 --- a/src/util/macvtap.c +++ b/src/util/macvtap.c @@ -1,4 +1,5 @@ /* + * Copyright (C) 2010 Red Hat, Inc. * Copyright (C) 2010 IBM Corporation * * This library is free software; you can redistribute it and/or @@ -48,9 +49,9 @@ #define VIR_FROM_THIS VIR_FROM_NET -#define ReportError(conn, code, fmt...) \ +#define ReportError(conn, code, ...) \ virReportErrorHelper(conn, VIR_FROM_NET, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) #define MACVTAP_NAME_PREFIXmacvtap #define MACVTAP_NAME_PATTERN macvtap%d diff --git a/src/util/pci.c b/src/util/pci.c index b812241..c6b2072 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -75,9 +75,9 @@ struct _pciDeviceList { /* For virReportOOMError() and virReportSystemError() */ #define VIR_FROM_THIS VIR_FROM_NONE -#define pciReportError(code, fmt...) \ +#define pciReportError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_NONE, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) /* Specifications referenced in comments: * PCI30 - PCI Local Bus Specification 3.0 diff --git a/src/util/stats_linux.c b/src/util/stats_linux.c index 25f8d08..feb5589 100644 --- a/src/util/stats_linux.c +++ b/src/util/stats_linux.c @@ -1,7 +1,7 @@ /* * Linux block and network stats. * - * Copyright (C) 2007-2009 Red Hat, Inc. + * Copyright (C) 2007-2010 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -28,9 +28,9 @@ #define VIR_FROM_THIS VIR_FROM_STATS_LINUX -#define virStatsError(code, fmt...)\ +#define virStatsError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_THIS, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) /* interface stats */ diff --git a/src/util/util.c b/src/util/util.c index cf7bba5..abead4a 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -85,9 +85,9 @@ #define VIR_FROM_THIS VIR_FROM_NONE -#define virUtilError(code, fmt...) \ +#define virUtilError(code, ...)\ virReportErrorHelper(NULL, VIR_FROM_NONE, code, __FILE__,
[libvirt] [PATCH] Change default for storage uid/gid from getuid()/getgid() to -1/-1
This allows the config to have a setting that means leave it alone, eg when building a pool where the directory already exists the user may want the current uid/gid of the directory left intact. This actually gets us back to older behavior - before recent changes to the pool building code, we weren't as insistent about honoring the uid/gid settings in the XML, and virt-manager was taking advantage of this behavior. As a side benefit, removing calls to getuid/getgid from the XML parsing functions also seems like a good idea. And having a default that is different from a common/useful value (0 == root) is a good thing in general, as it removes ambiguity from decisions (at least one place in the code was checking for (perms.uid == 0) to see if a special uid was requested). Note that this will only affect newly created pools and volumes. Due to the way that the XML is parsed, then formatted for newly created volumes, all existing pools/volumes already have an explicit uid and gid set. src/conf/storage_conf.c: Remove calls to setuid/setgid for default values of uid/gid, and set them to -1 instead src/storage/storage_backend.c: src/storage/storage_backend_fs.c: Make account for the new default values of perms.uid and perms.gid. --- src/conf/storage_conf.c |8 +++--- src/storage/storage_backend.c| 25 +-- src/storage/storage_backend_fs.c | 39 +++-- 3 files changed, 46 insertions(+), 26 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 19a1db9..f59f109 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -539,8 +539,8 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, if (node == NULL) { /* Set default values if there is not permissions element */ perms-mode = defaultmode; -perms-uid = getuid(); -perms-gid = getgid(); +perms-uid = -1; +perms-gid = -1; perms-label = NULL; return 0; } @@ -564,7 +564,7 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, } if (virXPathNode(./owner, ctxt) == NULL) { -perms-uid = getuid(); +perms-uid = -1; } else { if (virXPathLong(number(./owner), ctxt, v) 0) { virStorageReportError(VIR_ERR_XML_ERROR, @@ -575,7 +575,7 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, } if (virXPathNode(./group, ctxt) == NULL) { -perms-gid = getgid(); +perms-gid = -1; } else { if (virXPathLong(number(./group), ctxt, v) 0) { virStorageReportError(VIR_ERR_XML_ERROR, diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 3742493..ee6a32e 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -241,11 +241,10 @@ virStorageBackendCreateBlockFrom(virConnectPtr conn ATTRIBUTE_UNUSED, uid = (vol-target.perms.uid != st.st_uid) ? vol-target.perms.uid : -1; gid = (vol-target.perms.gid != st.st_gid) ? vol-target.perms.gid : -1; if (((uid != -1) || (gid != -1)) - (fchown(fd, vol-target.perms.uid, vol-target.perms.gid) 0)) { + (fchown(fd, uid, gid) 0)) { virReportSystemError(errno, _(cannot chown '%s' to (%u, %u)), - vol-target.path, vol-target.perms.uid, - vol-target.perms.gid); + vol-target.path, uid, gid); goto cleanup; } if (fchmod(fd, vol-target.perms.mode) 0) { @@ -356,10 +355,12 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup; } +uid_t uid = (vol-target.perms.uid == -1) ? getuid() : vol-target.perms.uid; +gid_t gid = (vol-target.perms.gid == -1) ? getgid() : vol-target.perms.gid; + if ((createstat = virFileOperation(vol-target.path, O_RDWR | O_CREAT | O_EXCL | O_DSYNC, - vol-target.perms.mode, - vol-target.perms.uid, vol-target.perms.gid, + vol-target.perms.mode, uid, gid, createRawFileOpHook, hdata, VIR_FILE_OP_FORCE_PERMS | (pool-def-type == VIR_STORAGE_POOL_NETFS @@ -491,14 +492,14 @@ cleanup: static int virStorageBuildSetUIDHook(void *data) { virStorageVolDefPtr vol = data; -if ((vol-target.perms.gid != 0) +if ((vol-target.perms.gid != -1) (setgid(vol-target.perms.gid) != 0)) { virReportSystemError(errno, _(Cannot set gid to %u before creating %s), vol-target.perms.gid, vol-target.path); return -1; } -if ((vol-target.perms.uid != 0) +if ((vol-target.perms.uid != -1)