- Devel - libvirt List Archives

[libvirt] [PATCH] Eliminate large stack buffer in doTunnelSendAll
by Laine Stump 06 Mar '10

06 Mar '10

doTunnelSendAll function (used by QEMU migration) uses a 64k buffer on the stack, which could be problematic. This patch replaces that with a buffer from the heap. While in the neighborhood, this patch also improves error reporting in the case that saferead fails - previously, virStreamAbort() was called (resetting errno) before reporting the error. It's been changed to report the error first. --- src/qemu/qemu_driver.c | 18 +++++++++++++++--- 1 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bb3edde..463c716 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8399,19 +8399,28 @@ cleanup: } +#define TUNNEL_SEND_BUF_SIZE 65536 + static int doTunnelSendAll(virStreamPtr st, int sock) { - char buffer[65536]; - int nbytes = sizeof(buffer); + char *buffer; + int nbytes = TUNNEL_SEND_BUF_SIZE; + + if (VIR_ALLOC_N(buffer, TUNNEL_SEND_BUF_SIZE) < 0) { + virStreamAbort(st); + virReportOOMError(); + return -1; + } /* XXX should honour the 'resource' parameter here */ for (;;) { nbytes = saferead(sock, buffer, nbytes); if (nbytes < 0) { - virStreamAbort(st); virReportSystemError(errno, "%s", _("tunnelled migration failed to read from qemu")); + virStreamAbort(st); + VIR_FREE(buffer); return -1; } else if (nbytes == 0) @@ -8421,10 +8430,13 @@ static int doTunnelSendAll(virStreamPtr st, if (virStreamSend(st, buffer, nbytes) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Failed to write migration data to remote libvirtd")); + VIR_FREE(buffer); return -1; } } + VIR_FREE(buffer); + if (virStreamFinish(st) < 0) /* virStreamFinish set the error for us */ return -1; -- 1.6.6.1

2 2

Re: [libvirt] [virt-devel] RFC: Modelling timers / clocks & tick policies in libvirt
by Zachary Amsden 06 Mar '10

06 Mar '10

On 03/05/2010 04:27 AM, Daniel P. Berrange wrote: > This mail describes how I'm suggesting libvirt addresses Dor's RFE to > timers/tick policies in libvirt > > https://bugzilla.redhat.com/show_bug.cgi?id=557285 > > Thanks to those who've answered my previous mail / irc questions. Please > point out any mistakes I made in understanding / modelling this problem > > Daniel > > > Virtual machine timer management in libvirt > =========================================== > > On PC hardware there are a number of terrible timers / clock sources available > to operating systems > > * PIT > Timer with periodic interrupts > > * RTC > > Time of Day clock, continuous running > Timer with periodic interrupts > > * Local APIC Timer > Timer with periodic interrupts > > * ACPI Timer > Timer with periodic interrupts > > * TSC > Read via rdtsc instruction. No interrupts > Never, ever tell anyone this, but it is possible to generate interrupts from the TSC. Doing so is a million orders of perverse because of the problems you list: > Unreliable on some hardware. eg changes frequency. > Not synced between cores > Different HZ across hosts > > * HPET > Multiple timers with periodic interrupts > Can replace PIT/RTC timers > > They all generally suck in real hardware, and this gets worse in virtual machines. > Many different approaches to making them suck less in VMWare, Xen& KVM, but there > are some reasonably common concepts.... > HPET doesn't suck. > > Virtual timekeeping problems > ---------------------------- > > Three primary problems / areas to deal with: > > * Time of day clock (RTC) > > - Initialized to UTC/Localtime/Timezone/UTC+offset > - Two modes of operation: > 1. Guest wallclock: only runs when guest is executing. ie stopped across save/restore, etc > 2. Host wallclock: runs continuously with host wall time. > Windows wants RTC in localtime, Linux can do either but RTC in UTC is easiest to maintain. Guest / host wall clock is a general consideration for all timers, obviously RTC is supposed to be a wallclock, but there is a question: do we allow the guest to see host time exactly or do we hide it? > * Interrupt timers > > - Ticks can not always be delivered on time > > Policies to deal with "missed" ticks: > > 1. Deliver at normal rate without catchup > 2. Deliver at higher rate to catch up > 3. Merge into 1 tick& deliver asap > > 4. Discard all missed ticks > The issue is actually more complex than just these policies. A naive implementation of the policy leads to a guest DOS of the host. We actually have such a bug, and it demands a policy which merges ticks over a certain threshold and does not deliver ASAP. It's tricky and complex to fix because it means our notion of timers for the guest is wrong, and we need to introduce a higher order scheduling behaviour. In general, there isn't much we can tune here, but what we can tune is whether the other counters (RTC / HPET / TSC / ACPI) stay in sync with ticks delivered. It's not perfect or completely well defined because the tick can't actually be delivered until a fairly complex set of hardware rules is obeyed. This may not be apparent now, because it gets worse as we implement more hardware support for NMIs and SMIs. An ideal solution would sync the other counters when the tick is generated, not when it is injected. However, this leads us back to the DOS attack. There are also problems with SMP timing here (which CPU gets timer interrupts can change, and are they broadcast?). These problems are made worse because we don't gang schedule. > * TSC > - rdtsc instruction can be exposed to guests in two ways > > 1. Trap + emulate (slow, but more reliable) > 2. Native (fast, but possibly unreliable) > > Optionally also expose a 'rdtscp' instruction > > Possiblly set a fixed HZ independant of host. > There is also 3) a mixed approach; trap and emulate only when required, allow native access and offset appropriately at each exit; and 4) a SMP safe approach; trap and emulate always, and interlock SMP access to the clock so it is globally consistent 5) a secure approach; trap and emulate always and hide host time. This precludes the possibility of SMP, as timing differences can be observed since we don't gang schedule. This obviously has implications for the other timers. So this variable is not a simple boolean, but a multi-choice. > VMWare timekeeping > ------------------ > > * All timers run in "apparant time" ie track guest wallclock > * Missed tick policy is to deliver at higher rate to catchup > * TSC can be switched between native/emulate (virtual_rdtsc=TRUE|FALSE) > * TSC can have hardcoded HZ in emulate mode (apparantHZ=VALUE) > * RTC time of day is synced to host at startup (rtc.diffFromUTC or rtc.startTime) > * VMWare tools reset guest TOD if it gets out of sync > There is also lateness hiding; (timeTracker.hideLateness); adjust TSC to compensate for lateness of injected interrupts (it's the slightly buggy counter compensation at each tick I mention above). > Xen timekeeping > --------------- > > * Virtual platform timer (VPT) used as source for other timers > * VPT has 4 modes > > 0: delay_for_missed_ticks > > Missed ticks are delivered when next scheduled, at the normal > rate. RTC runs in guest wallclock, so is delayed. No catchup is > attempted > > 1: no_delay_for_missed_ticks > > Missed ticks are delivered when next scheduled, at the normal > rate. RTC runs in host wallclock, so is not delayed. > > 2: no_missed_ticks_pending > > Missed ticks are discarded& next tick is delivered normally. RTC > runs in host wallclock. > > > 3: one_missed_tick_pending > > Missed interrupts are collapsed into a single late tick. RTC > run in host wallclock. > > * HPET > > Optionally enabled > > * TSC. Can run in 4 modes > > - auto: emulate if host TSC is unstable. native with invariant TSC > - native: always native regardless of host TSC stability > - emulate: trap + emulate regardless of host TSC invariant > - pvrdtsc: native, requiring invariant TSC. Also exposes rdtscp instruction > TSC is complex enough without RDTSCP. Let's consider rdtscp as a host optimization for vendors of hardware with buggy clocks who want fast gettimeofday system calls. We already are compensating to try to keep virtual TSC in sync on KVM and probably don't need this mode. > > KVM timekeeping > --------------- > > * PIT: can be in kernel, or userspace (userspace deprecated for KVM) > > Default tick policies differ for both impls > > - Userspace: Default: missed ticks are delivered when next scheduled at normal rate > > -tdf flag enable tick reinjection to catchup > > - Kernel: Default: Missed ticks are delivered at higher rate to catch up > > -no-kvm-pit-reinjection to disable tick reinjection catchup > > * RTC > > TOD clock can run in host or guest wallclock (clock=host|guest) > > Default: missed ticks are delivered when next scheduled at normal rate > > -rtc-td-hack or -clock driftfix=slew: missed ticks are delivered at a > higher rate to catchup > > * TSC > > - Always runs native. > > * HPET > > - Optionally enabled/disabled > > > Mapping in libvirt XML > ---------------------- > > Currently supports setting Time of Day clock via > > <clock offset="utc"/> > > Always sync to UTC > > <clock offset="localtime"/> > > Always sync to host timezone > > <clock offset="timezone" timezone='Europe/Paris'/> > > Sync to arbitrary timezone > > <clock offset="variable" adjustment='123456'/> > > Sync to UTC + arbitrary offset > > > > Proposal to model all timers policies as sub-elements of this<clock/> > In general we wil allow zero or more<timer/> elements following the > syntax: > > <timer name='platform|pit|rtc|hpet|tsc' > wallclock='host|guest' > tickpolicy='none|catchup|merge|discard' > frequency='123' > mode='auto|native|emulate|paravirt' > present='yes|no' /> > > Meaning of 'name': > > Names map to regular PC timers / clocks. 'Platform' refers to the > (optional) master virtual clock source that may be used to drive > policy of "other" clocks (eg used in Xen, which clocks are controlled > by the platform clock is to be undefined because it has varied in > Xen over time). > > Meaning of 'tickpolicy': > > none: continue to deliver at normal rate (ie ticks are delayed) > catchup: deliver at higher rate to catchup > merge: ticks merged into 1 single tick > discard: all missed ticks are discarded > > Meaning of 'wallclock': > > Only valid for name='rtc' or 'platform' > > host: RTC wallclock always tracks host time > guest: RTC wallclock always tracks host time > > Meaning of 'frequency': > > Set a fixed frequency in HZ. > > NB: Only relevant for TSC. All other timers are fixed (PIT, RTC), or > fully guest controlled frequency (HPET) > Actually, the guest doesn't control the HPET base frequency, only the divider. I think. I'd need to recheck the spec. > Meaning of 'mode': > > Control how the clock is exposed to guest. > > auto: native if safe, otherwise emulate > native: always native > emulate: always emulate > paravirt: native + paravirtualize > > NB: Only relevant for TSC. All other timers are always emulated. > auto, native, emulate can map nicely for us, but it would be good to have an smp safe mode. (A secure mode is more of a global setting for all timers). > Meaing of 'present': > > Used to override default set of timers visible to the guest. eg to > enable or disable the HPET > > > > Mapping to VMWare > ----------------- > > eg with guest config showing > > diffFromUTC='123456' > apparentHZ='123456' > virtual_rdtsc=False > > libvirt XML gets: > > <clock mode='variable' adjustment='123456'> > <timer name='tsc' frequency='123456' mode='native'/> > </clock> > > > Mapping to Xen > -------------- > > eg with guest config showing > > timer_mode=3 > hpet=1 > tsc_mode=2 > localtime=1 > > <clock mode='localtime'> > <timer name='platform' tickpolicy='merge' wallclock='host'/> > <timer name='hpet'/> > <timer name='tsc' mode='native'/> > </clock> > > > Mapping to KVM > -------------- > > eg with guest ARGV showing > > -no-kvm-pit-reinjection > -clock base=localtime,clock=guest,driftfix=slew > -no-hpet > > > <clock mode='localtime'> > <timer name='rtc' tickpolicy='catchup' wallclock='guest'/> > <timer name='pit' tickpolicy='none'/> > <timer name='hpet' present='no'/> > </clock> > > > > Further reading > --------------- > > VMWare has the best doc: > > http://www.vmware.com/pdf/vmware_timekeeping.pdf > > Xen: > > Docs on 'tsc_mode' at > > $SOURCETREE/docs/misc/tscmode.txt > > Docs for 'timer_mode' in the source code only: > > xen/include/public/hvm/params.h > > KVM: > > No docs at all. Guess from -help descriptions, reading source code& asking > clever people about it :-) > Let me propose an XML mapping a bit later today. I haven't had coffee yet, and we know what that can do. Zach

2 2

[libvirt] [PATCH 0/1] contributor guidelines
by David Allan 05 Mar '10

05 Mar '10

Here's a patch to consolidate the contributor guidelines into one place on the website and add a section on the use of goto. There was a small amount of content that was in HACKING but not in hacking.html.in, which I added. I also added Dan's suggested goto labels for common cases to the section on goto, and I removed the contents of HACKING and replaced them with a link to the website. Dave David Allan (1): Consolidating contributor guidelines HACKING | 395 +------------------------------------------------- docs/hacking.html.in | 125 ++++++++++++++++- 2 files changed, 125 insertions(+), 395 deletions(-)

2 2

[libvirt] [PATCH 1/3] xenXMDomainConfigParse: avoid dead store
by Jim Meyering 05 Mar '10

05 Mar '10

Here are three dead-store removals: >From f0645665521b07febb86f05778ca354126f01179 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 5 Mar 2010 16:15:09 +0100 Subject: [PATCH 1/3] xenXMDomainConfigParse: avoid dead store * src/xen/xm_internal.c (xenXMDomainConfigParse): Avoid dead store to local, "data". Remove declaration, too. --- src/xen/xm_internal.c | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 014cbfc..3710dcb 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -1,7 +1,7 @@ /* * xm_internal.h: helper routines for dealing with inactive domains * - * Copyright (C) 2006-2007, 2009 Red Hat + * Copyright (C) 2006-2007, 2009, 2010 Red Hat * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -1367,7 +1367,6 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { graphics->type = VIR_DOMAIN_GRAPHICS_TYPE_VNC; while (key) { - char *data; char *nextkey = strchr(key, ','); char *end = nextkey; if (nextkey) { @@ -1375,7 +1374,7 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { nextkey++; } - if (!(data = strchr(key, '='))) + if (!strchr(key, '=')) break; if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { -- 1.7.0.1.300.gd855a >From 78b1bea3186255891e48ddae1c822e21c5ff10db Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 5 Mar 2010 16:18:47 +0100 Subject: [PATCH 2/3] virInterfaceDefParseBond: avoid dead stores * src/conf/interface_conf.c (virInterfaceDefParseBond): Avoid dead stores to local, "node". Remove declaration, too. --- src/conf/interface_conf.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index a0d2dfa..f60b36a 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -570,7 +570,6 @@ error: static int virInterfaceDefParseBond(virInterfaceDefPtr def, xmlXPathContextPtr ctxt) { - xmlNodePtr node; int ret = -1; unsigned long tmp; @@ -582,8 +581,7 @@ virInterfaceDefParseBond(virInterfaceDefPtr def, if (ret != 0) goto error; - node = virXPathNode("./miimon[1]", ctxt); - if (node != NULL) { + if (virXPathNode("./miimon[1]", ctxt) != NULL) { def->data.bond.monit = VIR_INTERFACE_BOND_MONIT_MII; ret = virXPathULong("string(./miimon/@freq)", ctxt, &tmp); @@ -618,7 +616,7 @@ virInterfaceDefParseBond(virInterfaceDefPtr def, goto error; } - } else if ((node = virXPathNode("./arpmon[1]", ctxt)) != NULL) { + } else if (virXPathNode("./arpmon[1]", ctxt) != NULL) { def->data.bond.monit = VIR_INTERFACE_BOND_MONIT_ARP; -- 1.7.0.1.300.gd855a >From 197167ec109b2e69ad2a6a43d02f54dee457c120 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 5 Mar 2010 17:36:40 +0100 Subject: [PATCH 3/3] ebtablesAddRemoveRule: avoid dead store * src/util/ebtables.c (ebtablesAddRemoveRule): Avoid dead store to local, "s". --- src/util/ebtables.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/ebtables.c b/src/util/ebtables.c index dba9f82..988783c 100644 --- a/src/util/ebtables.c +++ b/src/util/ebtables.c @@ -1,6 +1,6 @@ /* * Copyright (C) 2009 IBM Corp. - * Copyright (C) 2007-2009 Red Hat, Inc. + * Copyright (C) 2007-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 @@ -185,7 +185,7 @@ ebtablesAddRemoveRule(ebtRules *rules, int action, const char *arg, ...) 1; /* arg */ va_start(args, arg); - while ((s = va_arg(args, const char *))) + while (va_arg(args, const char *)) n++; va_end(args); -- 1.7.0.1.300.gd855a

2 2

[libvirt] Libvirt segfault in qemuMonitorSend() with multi-threaded API use
by Adam Litke 05 Mar '10

05 Mar '10

I have a multi-threaded Python program that shares a single libvirt connection object among several threads (one thread per active domain on the system plus a management thread). On a heavily loaded host with 8 running domains I am getting a consistent libvirtd segfault in the qemu monitor handling code. This happens with libvirt-0.7.6 and git. Mar 4 12:23:13 bc1cn7-mgmt kernel: [ 3947.836151] libvirtd[7716]: segfault at 24 ip 000000000045de5c sp 00007fe5aa7d2b20 error 4 in libvirtd[400000+b3000] Using addr2line, this translates to: libvirt/src/qemu/qemu_monitor.c:698 Which is in qemuMonitorSend(): --> while (!mon->msg->finished) { if (virCondWait(&mon->notify, &mon->lock) < 0) goto cleanup; } It seems that mon->msg is being reset to NULL in the middle of this loop execution. I suspect that is because qemuMonitorSend() is not reentrant and multiple threads in my program are racing here. I would guess the 'mon->msg = NULL;' on line 707 causes the NULL that trips up the other racer. I presume the Monitor interface has some locking protection around it to ensure that only one thread can use it at a time? Is there an easy way to fix this? I am not familiar with the measures employed to make libvirt thread-safe. Thanks! -- Thanks, Adam

2 2

[libvirt] Update on the libvirt TCK status
by Daniel P. Berrange 05 Mar '10

05 Mar '10

A little while ago now I wrote about the initial libvirt TCK (Technology Compatability Kit): http://www.mail-archive.com/libvir-list@redhat.com/msg12703.html Since that time, it has moved on somewhat and I am regularly running it during development & immediately before release. It has caught a large number of bugs and was very helpful porting QEMU to the JSON mode monitor and -device syntax First and foremost, I moved the source repository to libvirt.org after we switched to GIT http://libvirt.org/git/?p=libvirt-tck.git The list of Perl module pre-requisites is still accurate as per the original mail. To simplify its use, it is now possible to run it on a system which you already have VMs on. The only restriction is that none of your virtual machines, storage pools, networks etc have a name that starts with the string prefix 'tck'. The test suite will complain & refuse to run if it finds any clashes. The --force option will tell it to blow away your existing VMs with names matching 'tck*'. I still, however, only recommend its use on machines that you don't care about. While I make reasonable effort not to trash your system, this kind of testing always has a fairly high level of risk / danger, either from bugs in libvirt, the HV, or the TCK itself. For many of the tests you can actually run within another virtual machine. eg, I have a Fedora 12 host, on which I run KVM. For running TCK tests, I installed a Fedora 12 guests, and then run the TCK in this. Obviously you won't be testing anything that requires hardware virt this way, but it is good enough for the vast majority of tests so far. It also makes it easy to setup tests against a large number of different OSs. It is no longer neccessary to manually download kernels/initrds. The test suite knows where to grab kernels/initrds off the interweb for Xen and any fullvirtualization. It uses the hypervisor capabilities data to determine the best kernels to use, and if it detects container based virt will setup a busybox chroot for testing instead. It will also create any disk images it needs, usually as sparse files, but sometimes as full files. The config file controls where these are created if you are short on space There are now somes tests which require access to hardware, so the config file allows the admin running the TCK to specify any disks, USB devices,m and PCI devices that can be used. If none are specified, any tests that require them are skipped automagically. Again, testing within a virtual machine makes this easy. Just add 4 extra PCI nics and a bunch of USB disks to your guests. You can then tell the TCK to play with them for its tests. The number of tests has increased sigificantly * domain/030-dominfo.t The virDomainGetInfo API works * domain/050-transient-lifecycle.t Basic lifecycle operations on transient guests * domain/051-transient-autostart.t Verify autostart is blocked on transient guests * domain/060-persistent-lifecycle.t Basic lifecycle operations on persistent guests * domain/061-persistent-autostart.t Verify autostart is allowed on persistent guests * domain/065-persistent-redefine.t Verify a persistent guest config can be updated * domain/070-transient-to-persistent.t Conversion of transient guests to persistent guests * domain/080-unique-id-define.t * domain/081-unique-id-create.t Verify name/uuid/id uniqueness when creating / defining guests * domain/090-invalid-ops-when-inactive.t Get suitabe errors for APIs which should not run on inactive guests * domain/100-transient-save-restore.t * domain/101-persistent-save-restore.t Save/restore to/from files for guests * domain/102-broken-save-restore.t Verify a nice error message for delibrately corrupt save files * domain/120-disks-stats.t The disk IO stats reporting * domain/200-disk-hotplug.t Basic hotplug of disks * domain/205-disk-hotplug-ordering.t Ordering of disks when hotplug different types (virtio/scsi) * domain/210-nic-hotplug.t Basic NIC hotplug * domain/215-nic-hotplug-many.t Hotplug of many NICs * domain/240-usb-host-hotplug.t USB hostdevice assignment to guests, by vendor/product and dev/bus * domain/250-pci-host-hotplug.t PCI hostdevice assignment to guests * qemu/100-disk-encryption.t A QEMU specific test for qcow2 disk encryption * storage/100-create-vol-dir.t Create of all types of files in a directory pool * storage/110-disk-pool.t Operation of the disk pool type (ie adding/removing partitions) * storage/200-clone-vol-dir.t Cloning of disk volumes There is of course scope for many many more tests to be added. Not least for storage we have NFS, iSCSI, SCSI, filesystem pools, LVM. The entire of the network interface APIs. Many more aspects of guest domain mgmt. Host devices, etc There is also work to be done to ensure the tests are fully portable across hypervisors. I've only really tested with QEMU/KVM, and to a lesser extent Xen and LXC. Running the tests is quite simple - Install the pre-requisite Perl modules (all in Fedora 12 and other distros) - Get a GIT checkout of the libvirt-tck - Build it perl ./Build.PL ./Build - Take the default config file conf/default.cfg in source tree, or5B /etc/libvirt-tck/default.cfg after installation and set the hypervisor URI. Optionally list some usb, pci & block devices if available for testing - Run it libvirt-tck -c /path/to/your/config.cfg The '-v' flag prints detailed progress info. The '-t' flag lets you point it to a single test script to avoid running all of them every time It is possible to run without installing it too export PERL5LIB=$TCK_GIT_TREE/lib perl $TCK_GIT_TREE/bin/libvirt-tck -c my.cfg -t $TCK_GIT_TREE/scripts 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 :|

1 0

[libvirt] Release of libvirt-0.7.7
by Daniel Veillard 05 Mar '10

05 Mar '10

So as planned, we are somehow a wekk late but February is short. Available at: ftp://libvirt.org/libvirt/ Despite that short month this includes a number of few features including new APIs and support for macvtap devices, a lot of bug fixes and a very large amount of cleanups in the code, result of Jim and Eric tracking plus cleanups in the internals interfaces by Matthias and Dan. Features: - Introduce public API for domain async job handling (Daniel P. Berrange) - macvtap support (Stefan Berger) - Add QEMU support for virtio channel (Matthew Booth) - Add persistence of PCI addresses to QEMU (Daniel P. Berrange) - Functions for computing baseline CPU from a set of host CPUs (Jiri Denemark) - Public API for virDomain{Attach,Detach}DeviceFlags (Jim Fehlig) Documentation: - web docs -- macvtap mode explanation (Stefan Berger) - Expand docs about clock modes (Daniel P. Berrange) - docs: Fix syntax warnings from recent changes. (Cole Robinson) - docs: network: Document <domain> element (Cole Robinson) - docs: network: Document STP and delay attributes (Cole Robinson) - docs: domain: Document <description> element (Cole Robinson) - docs: storage: Document multipath pools (Cole Robinson) - docs: storage: Document SCSI pools (Cole Robinson) - docs: storage: Fix backingStore <format> docs (Cole Robinson) - docs: storage: <volume><key> is always generated. (Cole Robinson) - docs: storage: Document capacity/alloc 'unit' (Cole Robinson) - docs: add 3 missing spaces (Dan Kenigsberg) - Fix typo in comment (Matthew Booth) - libvirt: Update docs for hotplug only commands (Cole Robinson) - Fix up a misspelled comment. (Chris Lalancette) - doc: restrict virDomain{Attach,Detach}Device to active domains (Jim Fehlig) - docs: Refer to virReportOOMError in the HACKING file (Matthias Bolte) - docs: Emphasize that devices have to be inside the <devices> element (Matthias Bolte) Portability: - build: vbox: avoid build failure when linking with --no-add-needed (Diego Elio Pettenò) - build: avoid dlopen-related link failure on rawhide/F13 (Diego Elio Pettenò) - Add a define for NFS_SUPER_MAGIC (Chris Lalancette) - Fix compliation of AppArmor related code (Matthias Bolte) Bug Fixes: - Fix USB passthrough based on product/vendor (Daniel P. Berrange) - Misc fixes for LXC cgroups setup (Daniel P. Berrange) - Change default for storage uid/gid from getuid()/getgid() to -1/-1 (Laine Stump) - Fix parser checking of storage pool device (Daniel P. Berrange) - Add missing device type check in QEMU PCI hotunplug (Daniel P. Berrange) - Make domain save work on root-squash NFS (Laine Stump) - Fix domain restore for files on root-squash NFS (Laine Stump) - Fix USB/PCI device address aliases in QEMU hotplug driver (Daniel P. Berrange) - Fix detection of errors in QEMU device_add command (Daniel P. Berrange) - uml: avoid crash on partial read (Eric Blake) - Fix QEMU domain state after a save attempt fails (Daniel P. Berrange) - Fix error messages when parsing USB devices in QEMU (Rolf Eike Beer) - Fix USB hotplug device string in QEMU driver (Rolf Eike Beer) - phypUUIDTable_Push: do not corrupt output stream upon partial write (Jim Meyering) - qemu: avoid null dereference on failed migration (Eric Blake) - Free the macvtap mode string (Stefan Berger) - libvirtd: do not ignore failure to set group ID in privileged mode (Jim Meyering) - Ignore SIGWINCH in remote client call to poll(2) (RHBZ#567931). (Richard Jones) - storage: conf: Correctly calculate exabyte unit (Cole Robinson) - virsh.c: avoid all leaks in OOM path in cmdCPUBaseline (Jiri Denemark) - Fixed reference count in virsh pool-build command (David Allan) - Fix daemon-conf invalid failures (David Allan) - virBufferVSprintf: do not omit va_end(argptr) call (Jim Meyering) - xend_internal.c: don't dereference NULL for unexpected input (Jim Meyering) - virsh: be careful to return "FALSE" upon OOM (Jim Meyering) - virBufferStrcat: do not skip va_end (Jim Meyering) - qparams.c: do not skip va_end, twice (Jim Meyering) - get_virtual_functions_linux: would mistakenly always return zero (Jim Meyering) - network: bridge: Fix IsActive, IsPersistent (Cole Robinson) - qemuMonitorTextAddUSBDisk: avoid unconditional leak (Jim Meyering) - tests: avoid NULL deref upon OOM failure (Jim Meyering) - qemuInitPasswords: avoid unconditional leak (Jim Meyering) - qemuMonitorTextAddDevice: avoid unconditional leak (Jim Meyering) - libvirt-override.c: avoid a leak upon call with invalid argument (Jim Meyering) - vboxDomainDumpXML: avoid a leak on OOM error path (Jim Meyering) - virNodeDevCapScsiHostParseXML: avoid an unconditional leak (Jim Meyering) - uml_driver.c: avoid leak upon failure (Jim Meyering) - vbox_tmpl.c: avoid an unconditional leak (Jim Meyering) - openvz (openvzFreeDriver): avoid leaks (Jim Meyering) - Fix crash in LXC driver open method when URI has no path (Daniel P. Berrange) - Fix USB device path formatting mixup (Daniel P. Berrange) - qemu_driver.c: honor dname parameter once again (Jim Meyering) - plug four virStoragePoolSourceFree-related leaks (Jim Meyering) - remote_driver.c: avoid leak on OOM error path (Jim Meyering) - qemu: Increase guest startup timeout to 30 seconds (Cole Robinson) - Fix security driver configuration (Daniel P. Berrange) - Escape strings serialized in XML (Daniel Veillard) - absolutePathFromBaseFile: don't leak when first arg contains no "/" (Jim Meyering) - sexpr_string: avoid leak on OOM error path (Jim Meyering) - virDomainChrDefParseXML: don't leak upon invalid input (Jim Meyering) - virExecWithHook: avoid leak on OOM error path (Jim Meyering) - cgroup.c: don't leak mem+FD upon OOM (Jim Meyering) - cgroup.c: avoid unconditional leaks (Jim Meyering) - virt-pki-validate contains unexpanded SYSCONFDIR variable (Doug Goldstein) Improvements: - Convert QEMU driver all hotunplug code from pci_del to device_del (Daniel P. Berrange) - Support hot-unplug for USB devices in QEMU (Daniel P. Berrange) - Tweak container initialization to make upstart/init happier (Daniel P. Berrange) - Avoid creating top level cgroups if just querying for existance (Daniel P. Berrange) - Support VCPU hotplug in QEMU guests (Daniel P. Berrange) - Fix mis-leading error message in pool delete API (Daniel P. Berrange) - Fix typo in QEMU migration command name (Daniel P. Berrange) - Don't raise error message from cgroups if QEMU fails to start (Daniel P. Berrange) - esx: don't ignore failure on close (Eric Blake) - Fix safezero() (Jiri Denemark) - Support job cancellation in QEMU driver (Daniel P. Berrange) - Remote driver implementation for the virDomainAbortJob APi (Daniel P. Berrange) - Wire up internal entry points for virDomainAbortJob API (Daniel P. Berrange) - Introduce public API for cancelling async domain jobs (Daniel P. Berrange) - Add QEMU driver support for job info on migration ops (Daniel P. Berrange) - Remote driver implmentation of job info API (Daniel P. Berrange) - Stub out internal driver entry points for job processing (Daniel P. Berrange) - Use device_del to remove SCSI controllers (Wolfgang Mauerer) - Fix PCI address handling when controllers are deleted (Wolfgang Mauerer) - Fix data structure handling when controllers are attached (Wolfgang Mauerer) - Allow configurable timezones with QEMU (Daniel P. Berrange) - Allow a timezone to be specified instead of sync to host timezone (Daniel P. Berrange) - Support variable clock offset mode in QEMU (Daniel P. Berrange) - Add new clock mode allowing variable adjustments (Daniel P. Berrange) - Change the internal domain conf representation of localtime/utc (Daniel P. Berrange) - Use standard spacing for user/pass prompt (Cole Robinson) - libvirtd: Better initscript error reporting (Cole Robinson) - qemu: Report binary path if error parsing -help (Cole Robinson) - remote: Improve daemon startup error reporting (Cole Robinson) - virsh: Show errors reported by nonAPI functions (Cole Robinson) - remote: Improve error message when libvirtd isn't running (Cole Robinson) - build: make git submodule checking more reliable (Jim Meyering) - Add descriptions for macvtap direct type interfaces (Stefan Berger) - maint: import modern bootstrap (Eric Blake) - maint: start factoring bootstrap (Eric Blake) - build: update gnulib submodule to latest (Jim Meyering) - Create raw storage files with O_DSYNC (again) (Jiri Denemark) - Use virFileOperation hook function in virStorageBackendFileSystemVolBuild (Laine Stump) - Rename virFileCreate to virFileOperation, add hook function (Laine Stump) - qemu: Check for IA64 kvm (Dustin Xiong) - remote: Print ssh stderr on connection failure (Cole Robinson) - fix multiple veth problem for OpenVZ (Yuji NISHIDA) - Better error reporting for failed migration (Chris Lalancette) - Make an error message in PCI util code clearer (Chris Lalancette) - macvtap mac_filter support (Stefan Berger) - macvtap IFF_VNET_HDR configuration (Stefan Berger) - Use virFork() in __virExec(), virFileCreate() and virDirCreate() (Laine Stump) - Add virFork() function to utils (Laine Stump) - Add domain support for virtio channel (Matthew Booth) - qemu: Explicitly error if guest virtual network is inactive (Cole Robinson) - virterror: Make SetError work if no previous error was set (Cole Robinson) - macvtap teardown rework (Stefan Berger) - Update QEMU JSON balloon command handling (Daniel P. Berrange) - python: Actually add virConnectGetVersion to generated bindings (Cole Robinson) - build: inform libtool of m4 directory (Eric Blake) - Adds a cpu-baseline command for virsh (Jiri Denemark) - qemu: Make SetVcpu command hotplug only (Cole Robinson) - qemu: Make Set*Mem commands hotplug only (Cole Robinson) - Treat missing QEMU 'thread_id' as non-fatal in JSON monitor (Daniel P. Berrange) - Fix check for primary IDE controller in QEMU PCI slot assignment (Daniel P. Berrange) - Make error reporting for QEMU JSON mode more friendly (Daniel P. Berrange) - Run 'qmp_capabilities' command at QEMU monitor startup (Daniel P. Berrange) - macvtap support for libvirt -- schema extensions (Stefan Berger) - macvtap support for libvirt -- qemu support (Stefan Berger) - macvtap support for libvirt -- helper code (Stefan Berger) - macvtap support for libvirt -- parse new interface XML (Stefan Berger) - interface: Use proper return codes in the open function (Matthias Bolte) - Support 'block_passwd' command for QEMU disk encryption (Daniel P. Berrange) - Implement cpuBaseline in remote and qemu drivers (Jiri Denemark) - Wire protocol format and dispatcher for virConnectBaselineCPU (Jiri Denemark) - virConnectBaselineCPU public API implementation (Jiri Denemark) - Internal driver API for virConnectBaselineCPU (Jiri Denemark) - virConnectBaselineCPU public API (Jiri Denemark) - Implement cpuArchBaseline in x86 CPU driver (Jiri Denemark) - Implement cpuArchBaseline in generic CPU driver (Jiri Denemark) - Mark all error messages for translation (Jiri Denemark) - Add cpu_generic.c to the list of translated files (Jiri Denemark) - Fix <cpu> element in domain XML schema (Jiri Denemark) - Fix disk stats retrieval with QEMU >= 0.12 (Daniel P. Berrange) - qemu: Properly report a startup timeout error (Cole Robinson) - test: Fake security driver support in capabilities (Cole Robinson) - Annotate some virConnectPtr as mandatory non-null (Daniel P. Berrange) - Convert qemu command line flags to 64-bit int (Daniel P. Berrange) - Create raw storage files with O_DSYNC (Jiri Denemark) - Re-generate remote protocol files for new APIs (Daniel P. Berrange) - Modify virsh commands (Jim Fehlig) - domain{Attach,Detach}DeviceFlags handler for drivers (Jim Fehlig) - Server side dispatcher (Jim Fehlig) - Remote driver (Jim Fehlig) - Wire protocol format (Jim Fehlig) - Public API Implementation (Jim Fehlig) Cleanups: - virsh: silence compiler warning (Eric Blake) - build: silence coverity warning in node_device (Eric Blake) - Tiny spelling fix (Wolfgang Mauerer) - libvirtd: avoid false-positive NULL-deref warning from clang (Eric Blake) - x86Decode: avoid NULL-dereference upon questionable input (Jim Meyering) - openvzDomainDefineCmd: remove useless increment (Jim Meyering) - maint: disallow TAB-in-indentation also in *.rng files (Jim Meyering) - maint: convert leading TABs in *.rng files to equivalent spaces (Jim Meyering) - udevEnumerateDevices: remove dead code (Jim Meyering) - qemudNetworkIfaceConnect: remove dead store (Jim Meyering) - cmdPoolDiscoverSources: initialize earlier to avoid FP from clang (Jim Meyering) - build: avoid warning about return-with-value in void function (Jim Meyering) - Only build virDomainObjFormat if not building proxy. (Chris Lalancette) - openvzGetVEID: don't leak (memory + file descriptor) (Jim Meyering) - build: avoid warning about unused variables (Jim Meyering) - build: avoid "make rpm" failure in docs/ (Jim Meyering) - build: teach apibuild.py to work in a non-srcdir build (Jim Meyering) - build: avoid non-srcdir "make distcheck" failures (CLEANFILES) (Jim Meyering) - build: avoid non-srcdir "make distcheck" failures (srcdir vs wildcard) (Jim Meyering) - build: avoid non-srcdir "make distcheck" failure (test_conf.sh) (Jim Meyering) - build: avoid non-srcdir installation failure (sitemap.html.in) (Jim Meyering) - build: avoid non-srcdir installation failure (apibuild.py) (Jim Meyering) - build: fix typos in makefile variable names (Jim Meyering) - build: ensure that MKINSTALLDIRS is AC_SUBST-defined (Jim Meyering) - maint: relax git minimum version (Eric Blake) - maint: sort .gitignore (Eric Blake) - maint: fix quoting in autogen.sh (Eric Blake) - virFork: placate static analyzers: ignore pthread_sigmask return value (Jim Meyering) - virsh.c: avoid leak on OOM error path (Jim Meyering) - Make virDomainObjFormat static (Chris Lalancette) - xenDaemonDomainSetAutostart: avoid appearance of impropriety (Jim Meyering) - Remove unused functions from domain_conf (Matthew Booth) - Fix whitespace in domain.rng (Matthew Booth) - openvzLoadDomains: don't ignore failing virUUIDFormat (Jim Meyering) - vshCommandParse: placate coverity (Jim Meyering) - virStorageBackendIsMultipath: avoid dead store (Jim Meyering) - Convert virSecurityReportError into a macro (Matthias Bolte) - Swap position of nmodels and models parameters in cpuDecode() (Jiri Denemark) - Remove virConnectPtr from secret XML APIs (Daniel P. Berrange) - Remove virConnectPtr from interface XML APIs (Daniel P. Berrange) - Remove virConnectPtr from CPU XML APIs (Daniel P. Berrange) - Remove virConnectPtr from storage APIs & driver (Daniel P. Berrange) - Remove virConnectPtr from all node device XML APIs (Daniel P. Berrange) - Remove virConnectPtr from network XML APis (Daniel P. Berrange) - Remove virConnectPtr from USB/PCI device iterators (Daniel P. Berrange) - Fix generation of floppy disk arg for QEMU's -global arg (Daniel P. Berrange) - Fix compile error in Xen proxy from virConnectPtr changes (Daniel P. Berrange) - Remove use of virConnectPtr from security driver APIs (Daniel P. Berrange) - Remove virConnectPtr from all domain XML parsing/formatting APIs (Daniel P. Berrange) - Remove virConnectPtr from LXC driver (Daniel P. Berrange) - Remove passing of virConnectPtr throughout QEMU driver (Daniel P. Berrange) - virAsprintf: remove its warn_unused_result attribute (Jim Meyering) - absolutePathFromBaseFile: avoid an unnecessary use of assert (Jim Meyering) - Remove conn parameter from USB functions (Matthias Bolte) - Remove conn parameter from JSON error macro (Matthias Bolte) - Remove conn parameter from PCI functions (Matthias Bolte) - Remove conn parameter from Linux stats functions (Matthias Bolte) - Remove conn parameter from storage file functions (Matthias Bolte) - Remove conn parameter from util functions (Matthias Bolte) - Remove conn parameter from virXPath* functions (Matthias Bolte) - Remove conn parameter from virReportSystemError (Matthias Bolte) - Remove conn parameter from virReportOOMError (Matthias Bolte) - website: Add a 1em right margin (Matthias Bolte) - storage: Replace storageLog with VIR_ERROR (Matthias Bolte) - opennebula: Remove unnecessary casts (Matthias Bolte) - esx: Remove unnecessary casts (Matthias Bolte) - cpu conf: Use virBufferFreeAndReset instead of virBufferContentAndReset and VIR_FREE (Matthias Bolte) - esx: Cleanup preprocessing structure in esxVI_EnsureSession (Matthias Bolte) Thanks everybody for getting this out ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

1 0

[libvirt] [PATCH] [REPOST] web docs -- macvtap mode explanation
by Stefan Berger 05 Mar '10

05 Mar '10

This adds more information about the different macvtap device modes, spells out VEPA and adds a link to a pdf at the ieee site. Regards, Stefan

2 1

[libvirt] [PATCH] qemuMonitorTextGetMemoryStats: decrease risk of false positive in parsing
by Jim Meyering 05 Mar '10

05 Mar '10

Not urgent. This was highlighted by clang as a dead store, since the first result stored in "offset" was never used. But if "info balloon" were ever to print some introductory text (containing a comma) before the balloon: actual... line, the bug would have made a difference. >From c81c6af87f20740a6b75652937ec8346f8bf59e3 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 5 Mar 2010 15:25:48 +0100 Subject: [PATCH] qemuMonitorTextGetMemoryStats: decrease risk of false positive in parsing The code erroneously searched the entire "reply" for a comma, when its intent was to search only that portion after "balloon: actual=" * src/qemu/qemu_monitor_text.c (qemuMonitorTextGetMemoryStats): Search for "," only starting *after* the BALLOON_PREFIX string. Otherwise, we'd be more prone to false positives. --- src/qemu/qemu_monitor_text.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 7f0e7f6..e629c6b 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -593,7 +593,8 @@ int qemuMonitorTextGetMemoryStats(qemuMonitorPtr mon, } if ((offset = strstr(reply, BALLOON_PREFIX)) != NULL) { - if ((offset = strchr(reply, ',')) != NULL) { + offset += strlen(BALLOON_PREFIX); + if ((offset = strchr(offset, ',')) != NULL) { ret = qemuMonitorParseExtraBalloonInfo(offset, stats, nr_stats); } } -- 1.7.0.1.300.gd855a

2 1

[libvirt] [PATCH] Fix USB passthrough based on product/vendor
by Daniel P. Berrange 05 Mar '10

05 Mar '10

Changeset commit 5073aa994af460e775cb3e548528e28d7660fcc8 Author: Cole Robinson <crobinso(a)redhat.com> Date: Mon Jan 11 11:40:46 2010 -0500 Added support for product/vendor based passthrough, but it only worked at the security driver layer. The main guest XML config was not updated with the resolved bus/device ID. When the QEMU argv refactoring removed use of product/vendor, this then broke launching guests. THe solution is to move the product/vendor resolution up a layer into the QEMU driver. So the first thing QEMU does is resolve the product/vendor to a bus/device and updates the XML config with this info. The rest of the code, including security drivers and QEMU argv generated can now rely on bus/device always being set. * src/util/hostusb.c, src/util/hostusb.h: Split vendor/product resolution code out of usbGetDevice and into usbFindDevice. Add accessors for bus/device ID * src/security/virt-aa-helper.c, src/security/security_selinux.c, src/qemu/qemu_security_dac.c: Remove vendor/product from the usbGetDevice() calls * src/qemu/qemu_driver.c: Use usbFindDevice to resolve vendor/product into a bus/device ID --- src/libvirt_private.syms | 3 + src/qemu/qemu_driver.c | 102 +++++++++++++++++++++++++++++++++++---- src/qemu/qemu_security_dac.c | 8 +-- src/security/security_selinux.c | 8 +-- src/security/virt-aa-helper.c | 4 +- src/util/hostusb.c | 39 +++++++++++---- src/util/hostusb.h | 13 +++-- 7 files changed, 136 insertions(+), 41 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ce9f013..c5ee23d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -599,7 +599,10 @@ virArgvToString; # usb.h usbGetDevice; +usbFindDevice; usbFreeDevice; +usbDeviceGetBus; +usbDeviceGetDevno; usbDeviceFileIterate; # uuid.h diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ef3cd6c..50bd55b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2283,17 +2283,15 @@ cleanup: return ret; } + static int -qemuPrepareHostDevices(struct qemud_driver *driver, - virDomainDefPtr def) +qemuPrepareHostPCIDevices(struct qemud_driver *driver, + virDomainDefPtr def) { pciDeviceList *pcidevs; int i; int ret = -1; - if (!def->nhostdevs) - return 0; - if (!(pcidevs = qemuGetPciHostDeviceList(def))) return -1; @@ -2344,6 +2342,62 @@ cleanup: return ret; } + +static int +qemuPrepareHostUSBDevices(struct qemud_driver *driver ATTRIBUTE_UNUSED, + virDomainDefPtr def) +{ + int i; + for (i = 0 ; i < def->nhostdevs ; i++) { + virDomainHostdevDefPtr hostdev = def->hostdevs[i]; + + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) + continue; + if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) + continue; + + /* Resolve a vendor/product to bus/device */ + if (hostdev->source.subsys.u.usb.vendor) { + usbDevice *usb + = usbFindDevice(hostdev->source.subsys.u.usb.vendor, + hostdev->source.subsys.u.usb.product); + + if (!usb) + return -1; + + hostdev->source.subsys.u.usb.bus = usbDeviceGetBus(usb); + hostdev->source.subsys.u.usb.device = usbDeviceGetDevno(usb); + + fprintf(stderr, "Resolve %u %u -> %u %u\n", + hostdev->source.subsys.u.usb.vendor, + hostdev->source.subsys.u.usb.product, + hostdev->source.subsys.u.usb.bus, + hostdev->source.subsys.u.usb.device ); + + usbFreeDevice(usb); + } + } + + return 0; +} + +static int +qemuPrepareHostDevices(struct qemud_driver *driver, + virDomainDefPtr def) +{ + if (!def->nhostdevs) + return 0; + + if (qemuPrepareHostPCIDevices(driver, def) < 0) + return -1; + + if (qemuPrepareHostUSBDevices(driver, def) < 0) + return -1; + + return 0; +} + + static void qemudReattachManagedDevice(pciDevice *dev) { @@ -6114,6 +6168,23 @@ static int qemudDomainAttachHostDevice(struct qemud_driver *driver, return -1; } + /* Resolve USB product/vendor to bus/device */ + if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB && + hostdev->source.subsys.u.usb.vendor) { + usbDevice *usb + = usbFindDevice(hostdev->source.subsys.u.usb.vendor, + hostdev->source.subsys.u.usb.product); + + if (!usb) + return -1; + + hostdev->source.subsys.u.usb.bus = usbDeviceGetBus(usb); + hostdev->source.subsys.u.usb.device = usbDeviceGetDevno(usb); + + usbFreeDevice(usb); + } + + if (driver->securityDriver && driver->securityDriver->domainSetSecurityHostdevLabel && driver->securityDriver->domainSetSecurityHostdevLabel(vm, hostdev) < 0) @@ -6677,11 +6748,22 @@ static int qemudDomainDetachHostUsbDevice(struct qemud_driver *driver, unsigned bus = vm->def->hostdevs[i]->source.subsys.u.usb.bus; unsigned device = vm->def->hostdevs[i]->source.subsys.u.usb.device; - - if (dev->data.hostdev->source.subsys.u.usb.bus == bus && - dev->data.hostdev->source.subsys.u.usb.device == device) { - detach = vm->def->hostdevs[i]; - break; + unsigned product = vm->def->hostdevs[i]->source.subsys.u.usb.product; + unsigned vendor = vm->def->hostdevs[i]->source.subsys.u.usb.vendor; + + if (dev->data.hostdev->source.subsys.u.usb.bus && + dev->data.hostdev->source.subsys.u.usb.device) { + if (dev->data.hostdev->source.subsys.u.usb.bus == bus && + dev->data.hostdev->source.subsys.u.usb.device == device) { + detach = vm->def->hostdevs[i]; + break; + } + } else { + if (dev->data.hostdev->source.subsys.u.usb.product == product && + dev->data.hostdev->source.subsys.u.usb.vendor == vendor) { + detach = vm->def->hostdevs[i]; + break; + } } } diff --git a/src/qemu/qemu_security_dac.c b/src/qemu/qemu_security_dac.c index f281b96..6911f48 100644 --- a/src/qemu/qemu_security_dac.c +++ b/src/qemu/qemu_security_dac.c @@ -206,9 +206,7 @@ qemuSecurityDACSetSecurityHostdevLabel(virDomainObjPtr vm, switch (dev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: { usbDevice *usb = usbGetDevice(dev->source.subsys.u.usb.bus, - dev->source.subsys.u.usb.device, - dev->source.subsys.u.usb.vendor, - dev->source.subsys.u.usb.product); + dev->source.subsys.u.usb.device); if (!usb) goto done; @@ -277,9 +275,7 @@ qemuSecurityDACRestoreSecurityHostdevLabel(virDomainObjPtr vm ATTRIBUTE_UNUSED, switch (dev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: { usbDevice *usb = usbGetDevice(dev->source.subsys.u.usb.bus, - dev->source.subsys.u.usb.device, - dev->source.subsys.u.usb.vendor, - dev->source.subsys.u.usb.product); + dev->source.subsys.u.usb.device); if (!usb) goto done; diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 06a2479..b2c8581 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -491,9 +491,7 @@ SELinuxSetSecurityHostdevLabel(virDomainObjPtr vm, switch (dev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: { usbDevice *usb = usbGetDevice(dev->source.subsys.u.usb.bus, - dev->source.subsys.u.usb.device, - dev->source.subsys.u.usb.vendor, - dev->source.subsys.u.usb.product); + dev->source.subsys.u.usb.device); if (!usb) goto done; @@ -561,9 +559,7 @@ SELinuxRestoreSecurityHostdevLabel(virDomainObjPtr vm, switch (dev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: { usbDevice *usb = usbGetDevice(dev->source.subsys.u.usb.bus, - dev->source.subsys.u.usb.device, - dev->source.subsys.u.usb.vendor, - dev->source.subsys.u.usb.product); + dev->source.subsys.u.usb.device); if (!usb) goto done; diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 066e18b..78bef41 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -836,9 +836,7 @@ get_files(vahControl * ctl) switch (dev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: { usbDevice *usb = usbGetDevice(dev->source.subsys.u.usb.bus, - dev->source.subsys.u.usb.device, - dev->source.subsys.u.usb.vendor, - dev->source.subsys.u.usb.product); + dev->source.subsys.u.usb.device); if (usb == NULL) continue; diff --git a/src/util/hostusb.c b/src/util/hostusb.c index bf96539..afba325 100644 --- a/src/util/hostusb.c +++ b/src/util/hostusb.c @@ -159,9 +159,7 @@ cleanup: usbDevice * usbGetDevice(unsigned bus, - unsigned devno, - unsigned vendor, - unsigned product) + unsigned devno) { usbDevice *dev; @@ -170,14 +168,6 @@ usbGetDevice(unsigned bus, return NULL; } - if (vendor) { - /* Look up bus.dev by vendor:product */ - if (usbFindBusByVendor(vendor, product, &bus, &devno) < 0) { - VIR_FREE(dev); - return NULL; - } - } - dev->bus = bus; dev->dev = devno; @@ -194,6 +184,21 @@ usbGetDevice(unsigned bus, return dev; } + +usbDevice * +usbFindDevice(unsigned vendor, + unsigned product) +{ + unsigned bus = 0, devno = 0; + + if (usbFindBusByVendor(vendor, product, &bus, &devno) < 0) { + return NULL; + } + + return usbGetDevice(bus, devno); +} + + void usbFreeDevice(usbDevice *dev) { @@ -202,6 +207,18 @@ usbFreeDevice(usbDevice *dev) } +unsigned usbDeviceGetBus(usbDevice *dev) +{ + return dev->bus; +} + + +unsigned usbDeviceGetDevno(usbDevice *dev) +{ + return dev->dev; +} + + int usbDeviceFileIterate(usbDevice *dev, usbDeviceFileActor actor, void *opaque) diff --git a/src/util/hostusb.h b/src/util/hostusb.h index bc22671..9a1b1b7 100644 --- a/src/util/hostusb.h +++ b/src/util/hostusb.h @@ -26,11 +26,14 @@ typedef struct _usbDevice usbDevice; -usbDevice *usbGetDevice (unsigned bus, - unsigned devno, - unsigned vendor, - unsigned product); -void usbFreeDevice (usbDevice *dev); +usbDevice *usbGetDevice(unsigned bus, + unsigned devno); +usbDevice *usbFindDevice(unsigned vendor, + unsigned product); +void usbFreeDevice (usbDevice *dev); + +unsigned usbDeviceGetBus(usbDevice *dev); +unsigned usbDeviceGetDevno(usbDevice *dev); /* * Callback that will be invoked once for each file -- 1.6.5.2

2 1