[libvirt] [PATCH 0/4] qemu: Fix leaks in handling PCI devices
by Jiri Denemark
https://bugzilla.redhat.com/show_bug.cgi?id=877095
Patch 1/4 fixes a possible double free on error path and the other patches
deal with memory and file descriptor leaks in PCI device attachment code.
Jiri Denemark (4):
qemu: Don't free PCI device if adding it to activePciHostdevs fails
util: Slightly refactor PCI list functions
qemu: Fix memory (and FD) leak on PCI device detach
qemu: Do not keep PCI device config file open
src/libvirt_private.syms | 3 +++
src/qemu/qemu_hostdev.c | 22 ++++++++--------
src/util/pci.c | 66 ++++++++++++++++++++++++++++--------------------
src/util/pci.h | 5 ++++
4 files changed, 58 insertions(+), 38 deletions(-)
--
1.8.0
12 years
[libvirt] virSecurity hook for hugepages?
by Serge Hallyn
Hi,
Currently the hugepages support can automatically detect the hugepages
mount, but it doesn't update the security information. At least for
apparmor we need to be able to add permission for the domain to access
the hugetlbfs mount path.
There are a few ways this could be done,
1. add a virSecuritySetSecurityHugepages or virSecuritySetSecurityHugepagesFD
hook which is called perhaps at qemudStartup
2. optionally add the qemu_driver->hugepage_path to the xml output, at
least for the internal format (which is passed to virt-aa-helper). The
concern I have with this is that it brings up the issue of what to do
when defining a domain which has such an entry.
3. reproduce the logic in virt-aa-helper for detecting the hugepages
mount path. Not preferred obviously.
My guess would be that (1) would be preferred, but I wanted to ask here
first and see if there are other suggestions.
thanks,
-serge
12 years
[libvirt] [PATCH] storage: Error out earlier if the volume target path already exists
by Osier Yang
https://bugzilla.redhat.com/show_bug.cgi?id=832302
It's odd to fall through to buildVol, and the existed file is
removed when buildVol fails. This checks if the volume target
path already exists in createVol. The reason for not using
error like "Volume already exists" is that there isn't volume
maintained by libvirt for the path until a operation like
pool-refresh, using error like that will just cause confusion.
---
BTW, It inspires me again that perhaps we should integrate things
like inotify to storage.
---
src/storage/storage_backend_fs.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 4e6ebbf..6cddad0 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1000,6 +1000,13 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn ATTRIBUTE_UNUSED,
return -1;
}
+ if (virFileExists(vol->target.path)) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ _("volume target path '%s' already exists"),
+ vol->target.path);
+ return -1;
+ }
+
VIR_FREE(vol->key);
vol->key = strdup(vol->target.path);
if (vol->key == NULL) {
--
1.7.7.6
12 years
[libvirt] [PATCH 0/2] Avoid the thread race condition
by Osier Yang
https://bugzilla.redhat.com/show_bug.cgi?id=866524
The two patches are to fix the same bug in different method,
1/2 is suggested by Daniel in the bug. 2/2 is an alternative
way to fix it by locking the whole virConnect object.
The reason to post 2/2 is that I think 1/2 doesn't fix
the root problem, though I believe it must fix the bug. A
thread can get the lock before/during doRemoteClose, and it
might drop into the similar situation, though it's not the
case now because remoteClientCloseFunc doesn't do changes
which can cause race for the later doRemoteClose. However,
I'm not sure if locking the whole object will cause some
unexpected result.
Osier Yang (2):
remote: Avoid the thread race condition
Lock the whole virConnect Object when disposing to avoid the thread
race
src/datatypes.c | 4 ++--
src/remote/remote_driver.c | 4 ++++
2 files changed, 6 insertions(+), 2 deletions(-)
Regards,
Osier
12 years
Re: [libvirt] [libvirt-users] NWFilter and IPv6
by Laine Stump
(No extra content from me, but I'm setting Followup-To:
libvir-list(a)redhat.com for this (and setting To: to the same), since
this is talking about new development, so we want to make sure as many
developers as possible see it...)
On 12/05/2012 09:23 AM, Guido Winkelmann wrote:
> Am Dienstag, 4. Dezember 2012, 19:18:01 schrieb Stefan Berger:
>> On 12/04/2012 09:39 AM, Guido Winkelmann wrote:
>>> Am Montag, 26. November 2012, 12:24:11 schrieb Stefan Berger:
>>>> On 11/26/2012 10:41 AM, Laine Stump wrote:
> [...]
>>>> One problem I want to mention, though: A bigger problem would be if a
>>>> machine wanted to use IPv4 and IPv6 (dual stack) and use DHCP for both ,
>>>> which in effect would result in two variables that need to have values
>>>> detected which in turn would require partial instantiation of filters
>>>> (since one variable may not have a value assigned while the other has),
>>>> which does not currently work...
>>> Hm, how do you even do it with one variable? Do you leave the firewall
>>> undefined until you could detect the dhcp-answer package and then pull it
>>> up?
>> We assume that DHCP is being used and for example put a filter in that
>> only allows DHCP traffic to pass and once we grab the IP address we
>> instantiate the user-provided filter. For that we use $IP. The variable
>> is set once the IP address has been detected. For IPv6 we should
>> probably use $IPV6 (reserved variable).
> How do you control this behavior? Can you just set the $IP to a value in the
> filterref instantiation to disable it? Like so:
>
> <filterref filter='clean-traffic-with-v6'>"
> <parameter name='MAC' value='11:11:11:11:11:11'/>";
> <parameter name='IP' value='192.168.0.10'/>";
> <parameter name='IP' value='192.168.0.11'/>";
> </filterref>"
>
> Anyway, I think combining DHCP and DHCPv6 is going to be a minor problem in
> practice, because most people will probably use stateless autoconfiguration to
> set the IPv6 address on a device and use DHCPv6 only for additional
> information, like DNS servers or NTP servers.
>
> How about this approach instead for combining DHCP and DHCPv6:
>
> - Instead of putting up a special network filter for the detection phase, we
> put up the actual user-requested filter, but with $IP and $IPV6 unset. (except
> of course when these variables are specifically set by the user as above...)
> - The default-shipped filters like clean-traffic should let DHCP(v6) through
> in this configuration. If the user-requested filters don't, that's just a
> configuration error.
> - As soon as we detect an incoming DHCP or DHCPv6 packet for the guest, we add
> that address to the filter parameters, reinitialize the filter with the new
> parameters and stop detecting addresses for this particular L3 protocol.
>
>>>> Also as I recall for IPv4 the ARP-equivalent is NDP (Neighbor Discovery
>>>> Protocol based on ICMPv6), which may need support in ebtables. At least
>>>> a while ago there was no support for filtering that NDP subset of ICMPv6
>>>> in ebtables.
>>> According to the ebtables man-page, you've got --ip6-icmp-type, which
>>> should be enough for this. Router advertisements have ICMPv6 type 134 and
>>> multicast router advertisements are 153. AFAICT, you can just filter by
>>> those...
>> I am not the expert on IPv6, but from reading on this page here
>>
>> http://www.tcpipguide.com/free/t_ICMPv6NeighborAdvertisementandNeighborSolic
>> itation-2.htm
> BTW, I wouldn't recommend this particular guide. Not only is it cluttered with
> advertisements to the point of a major annoyance, there's at least one part
> where it has simply plain wrong information: On
>
> http://www.tcpipguide.com/free/t_IPv6InterfaceIdentifiersandPhysicalAddre...
>
> Bit 7 of the EUI-64 address needs to be flipped, not just set to 1. (It took
> me a while to figure out why my code would arrvive at different autoconfigured
> addresses than the linux kernel for virtualized machines...)
>
>> I get the impression that for example the target address should be
>> verified for possible 'abuse'.
> That's only for neighbor advertisements. Router advertisements can simply be
> blocked wholesale. Normal network nodes have no business sending those under
> any circumstances, and your actual routers are hopefully trusted enough to not
> need their router advertisements checked for sanity...
>
> Then again, for neighbor advertisements, you're right. I was under the
> impression that, for those, it was enough to check the source address in the
> ipv6 header. Apparently I was wrong.
>
>> I don't think one can grab that field
>> with ebtables and compare against allowed values.
> No, but ip6tables has --u32, which possibly could be abused for that...
>
> Guido
>
>
12 years
[libvirt] Sophie's choice: corrupt user data (on USB disk) or cancel migration?
by David Jaša
Hi,
I've encountered an interesting problem: I did a live migration through
libvirt (virsh migrate --live <domain> <dst_libvirt_uri>
[<dst_qemu_uri>]) with usb transfer ongoing from qemu guest through
usbredir over spice to usb device plugged to the client. The spice
client failed to connect to the destination qemu/spice but the migration
was conducted anyway, so the data transfer was abruptly terminated.
Background
----------
When performing migration with client connected, two monitor commands
are issued to the qemu:
(qemu) client_migrate_info spice <client connection details>
main_channel_client_handle_migrate_connected: client <ID> connected: 1
seamless: 1
(qemu) migrate -d tcp:<dst_qemu_address>:<port>
The first command instructs spice client to reconnect to the new machine
and the reply says that if connection is successful and if it is
seamless (required for migration with USB redirection active).
Problem
-------
The behaviour should follow "least harm" principle:
1) In "severe" scenarios when the choice is between VM migration causing
potential data corruption on redirected USB and stopping of the VM,
current behavior is perfectly fine
2) In "regular" scenarios (automatic load balancing, manual migration
requests by administrator), refuse to migrate (unless some "I really
know what I'm doing" button is pressed) is better thing to do
Possible solutions?
-------------------
Just one possible way to fix this occured to me:
1. libvirt gets new switch to migrate command to force migration even
when client can not connect
2. if this switch is not set and client won't connect after
client_migrate_info qemu command is issued, cance migration
3. management application (ovirt-engine through vdsm) or administrator
sets the switch to force migration in cases like 1) above
I'll file the necessary bugs but it should be IMO first agreed what the
correct approach is in such situations.
David
--
David Jaša, RHCE
SPICE QE based in Brno
GPG Key: 22C33E24
Fingerprint: 513A 060B D1B4 2A72 7F0D 0278 B125 CD00 22C3 3E24
12 years
[libvirt] [PATCH] pci: Fix building of 32bit PCI command array
by Peter Krempa
The pciWrite32 function assembled the array of data to be written to the
fd with a bad offset on the last byte. This issue was probably caused by
a typo (14, 24).
---
src/util/pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/pci.c b/src/util/pci.c
index cf7722d..418c83d 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -268,7 +268,7 @@ pciWrite16(pciDevice *dev, int cfgfd, unsigned pos, uint16_t val)
static void
pciWrite32(pciDevice *dev, int cfgfd, unsigned pos, uint32_t val)
{
- uint8_t buf[4] = { (val >> 0), (val >> 8), (val >> 16), (val >> 14) };
+ uint8_t buf[4] = { (val >> 0), (val >> 8), (val >> 16), (val >> 24) };
pciWrite(dev, cfgfd, pos, &buf[0], sizeof(buf));
}
--
1.8.0
12 years
[libvirt] [PATCH] qemu: Fix error code when attaching existing device
by Jiri Denemark
An attempt to attach device that is already attached to a domain results
in the following error:
virsh # attach-device rhel6 pci2 --persistent
error: Failed to attach device from pci2
error: invalid argument: device is already in the domain configuration
The "invalid argument" error code looks wrong, we usually use "operation
invalid" when the action cannot be done in current state.
---
src/qemu/qemu_driver.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8e838cd..6170ded 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6249,8 +6249,8 @@ qemuDomainAttachDeviceConfig(qemuCapsPtr caps,
case VIR_DOMAIN_DEVICE_DISK:
disk = dev->data.disk;
if (virDomainDiskIndexByName(vmdef, disk->dst, true) >= 0) {
- virReportError(VIR_ERR_INVALID_ARG,
- _("target %s already exists."), disk->dst);
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ _("target %s already exists"), disk->dst);
return -1;
}
if (virDomainDiskInsert(vmdef, disk)) {
@@ -6280,7 +6280,7 @@ qemuDomainAttachDeviceConfig(qemuCapsPtr caps,
case VIR_DOMAIN_DEVICE_HOSTDEV:
hostdev = dev->data.hostdev;
if (virDomainHostdevFind(vmdef, hostdev, NULL) >= 0) {
- virReportError(VIR_ERR_INVALID_ARG, "%s",
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("device is already in the domain configuration"));
return -1;
}
@@ -6296,7 +6296,7 @@ qemuDomainAttachDeviceConfig(qemuCapsPtr caps,
case VIR_DOMAIN_DEVICE_LEASE:
lease = dev->data.lease;
if (virDomainLeaseIndex(vmdef, lease) >= 0) {
- virReportError(VIR_ERR_INVALID_ARG,
+ virReportError(VIR_ERR_OPERATION_INVALID,
_("Lease %s in lockspace %s already exists"),
lease->key, NULLSTR(lease->lockspace));
return -1;
@@ -6312,7 +6312,7 @@ qemuDomainAttachDeviceConfig(qemuCapsPtr caps,
controller = dev->data.controller;
if (virDomainControllerFind(vmdef, controller->type,
controller->idx) > 0) {
- virReportError(VIR_ERR_INVALID_ARG, "%s",
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("Target already exists"));
return -1;
}
--
1.8.0
12 years
[libvirt] Set python include dir when cross-compiling
by Prica, Mihai
Hi,
I've done some work on libvirt integration in the yocto project. The problem is there is no way to tell the configure script which include_dir to use for the python bindings when cross-compiling. It automatically takes the include_dir from the host system, and if there is a difference in architecture between the target and host system, the build will fail.
Is there any easy way of doing this? I saw for example that rpm has two extra options in the configure file, --with-python-inc-dir and -with-python-lib-dir exactly for this.
Thanks,
Mihai
12 years
[libvirt] [PATCH 00/15 v5] Unprivileged SG_IO support
by Osier Yang
Hi,
As a result of RFC [1], this implements the unprivleged SG_IO
support. Testing is not that enough, but I'd like see the
reviewing earlier, and meanwhile I'm not going to give up
the further testing.
v4 - v5 (5 new patches):
* Set sysfs unpriv_sgio when attaching disk
* Restore sysfs unpriv_sgio when detaching disk
* Error out when attaching disk if it's shared by other
(domains), and the disk conf conflicts.
* Do not restore sysfs unpriv_sgio when detaching disk
if the disk is still being used by other domain(s)
* Dump the original unpriv_sgio state in status XML,
so that it won't be lost after restarting or reloading
libvirtd.
v3 - v4:
* Rebase on the top
* More testing
v2 - v3:
* Change the XML tag name to "cdbfilter"
* Maintain an internal list of shared disks for QEMU driver.
Patches 1/10 ~ 4/10 are to introduce the internal list for shared
disks.
Osier Yang (15):
qemu: Introduce a list to maintain the shared disks between domains
qemu: Init/Free the list with the driver's lifecyle
qemu: Add/remove the shared disk entry during domain's lifecyle
qemu: Add/Remove the entry of sharedDisks when live
attaching/detaching
docs: Add docs and rng schema for new XML cdbfilter
conf: Parse and format the new XML tag cdbfilter
util: Prepare helpers for unpriv_sgio setting
qemu: Manage disk's cdbfilter in domain's lifecycle
qemu: Do not restore the sysfs unpriv_sgio if the disk is being
shared
qemu: Error out when domain starting if the cdbfilter setting
conflicts
qemu: Set unpriv_sgio when attaching disk
qemu: Restore unpriv_sgio when detaching disk
qemu: Error out if the shared disk conf conflicts with others when
attaching
qemu: Do not restore unpriv_sgio if the disk is shared by other
domain
conf: Save disk's original unpriv_sgio state into status XML
docs/formatdomain.html.in | 13 ++-
docs/schemas/domaincommon.rng | 52 ++++--
src/conf/domain_conf.c | 106 ++++++++++--
src/conf/domain_conf.h | 13 ++
src/libvirt_private.syms | 5 +
src/qemu/qemu_conf.c | 170 ++++++++++++++++++++
src/qemu/qemu_conf.h | 30 ++++
src/qemu/qemu_driver.c | 78 +++++++++
src/qemu/qemu_process.c | 141 ++++++++++++++++-
src/qemu/qemu_process.h | 4 +
src/util/util.c | 145 +++++++++++++++++
src/util/util.h | 7 +
...ml2argv-disk-scsi-lun-passthrough-cdbfilter.xml | 32 ++++
tests/qemuxml2xmltest.c | 1 +
14 files changed, 761 insertions(+), 36 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough-cdbfilter.xml
Regards,
Osier
12 years
