- Devel - libvirt List Archives

[libvirt] [PATCH] Add TCK script for verifying disk based storage pools
by Daniel P. Berrange 04 Mar '10

04 Mar '10

This adds a reasonably comprehensive test script for working with disk based storage pools. It checks - Prevent re-formating an existing partition table (currently missing this check in libvirt) - Formating of a partiton table - Wiping a partition table (currently missing in libvirt) - Creating multiple storage volumes - Deleteing storage volumes * conf/default.cfg: add config entry for specifying host block devices that a test script can use * lib/Sys/Virt/TCK.pm: Add API to get a host block device * lib/Sys/Virt/TCK/StoragePoolBuilder.pm: Fix typo in device XML formatting. Allow pool source format to be specified * scripts/storage/110-disk-pool.t: Test which creates a storage pool, creates some volumes, then deletes it all again. --- conf/default.cfg | 9 ++ lib/Sys/Virt/TCK.pm | 7 ++ lib/Sys/Virt/TCK/StoragePoolBuilder.pm | 13 +++- scripts/storage/110-disk-pool.t | 127 ++++++++++++++++++++++++++++++++ 4 files changed, 155 insertions(+), 1 deletions(-) create mode 100644 scripts/storage/110-disk-pool.t diff --git a/conf/default.cfg b/conf/default.cfg index 5b85cbc..288540f 100644 --- a/conf/default.cfg +++ b/conf/default.cfg @@ -127,3 +127,12 @@ host_pci_devices = ( # } # Can list more than one PCI device if many are available ) + +# List of host block devices that the test suite can trash all +# data on, for purposes of testing. You have been warned ! +# Only full block devices allowed, no single partitions, since +# the test suite itself needs to create partitions. +# The disks should be at *least* 512 MB in size +host_block_devices = ( +# /dev/vdb +) diff --git a/lib/Sys/Virt/TCK.pm b/lib/Sys/Virt/TCK.pm index f101937..d32b03d 100644 --- a/lib/Sys/Virt/TCK.pm +++ b/lib/Sys/Virt/TCK.pm @@ -805,4 +805,11 @@ sub get_host_pci_device { return ($domain, $bus, $slot, $function); } +sub get_host_block_device { + my $self = shift; + my $devindex = @_ ? shift : 0; + + return $self->config("host_block_devices/[$devindex]", undef); +} + 1; diff --git a/lib/Sys/Virt/TCK/StoragePoolBuilder.pm b/lib/Sys/Virt/TCK/StoragePoolBuilder.pm index f92b21a..c9ef2dd 100644 --- a/lib/Sys/Virt/TCK/StoragePoolBuilder.pm +++ b/lib/Sys/Virt/TCK/StoragePoolBuilder.pm @@ -77,6 +77,14 @@ sub target { return $self; } +sub format { + my $self = shift; + + $self->{format} = shift; + + return $self; +} + sub as_xml { my $self = shift; @@ -98,7 +106,7 @@ sub as_xml { } if ($self->{source}->{device}) { foreach my $dev (@{$self->{source}->{device}}) { - $w->emptyTag("dev", path => $dev); + $w->emptyTag("device", path => $dev); } } if ($self->{source}->{adapter}) { @@ -107,6 +115,9 @@ sub as_xml { if ($self->{source}->{name}) { $w->dataElement("name", $self->{source}->{name}); } + if ($self->{format}) { + $w->emptyTag("format", type => $self->{format}); + } $w->endTag("source"); $w->startTag("target"); diff --git a/scripts/storage/110-disk-pool.t b/scripts/storage/110-disk-pool.t new file mode 100644 index 0000000..faa13c5 --- /dev/null +++ b/scripts/storage/110-disk-pool.t @@ -0,0 +1,127 @@ +# -*- perl -*- +# +# Copyright (C) 2009-2010 Red Hat +# Copyright (C) 2009-2010 Daniel P. Berrange +# +# This program is free software; You can redistribute it and/or modify +# it under the GNU General Public License as published by the Free +# Software Foundation; either version 2, or (at your option) any +# later version +# +# The file "LICENSE" distributed along with this file provides full +# details of the terms and conditions +# + +=pod + +=head1 NAME + +storage/110-disk-pool.t - test disk storage pools + +=head1 DESCRIPTION + +The test case validates that it is possible to use all core +functions of the disk pool type. + +=cut + +use strict; +use warnings; + +use Test::More tests => 20; + +use Sys::Virt::TCK; +use Test::Exception; +use File::stat; + +my $tck = Sys::Virt::TCK->new(); +my $conn = eval { $tck->setup(); }; +BAIL_OUT "failed to setup test harness: $@" if $@; +END { + $tck->cleanup if $tck; +} + + +SKIP: { + my $dev = $tck->get_host_block_device(); + + skip "no host block device available", 20 unless defined $dev; + + # Blow away partition table (if any) + open DEV, ">$dev" or die "cannot write to $dev: $!"; + my $data = " "x 512; + + print DEV $data; + close DEV or die "cannot save $dev: $!"; + + my $poolxml = $tck->generic_pool("disk", "tck") + ->source_device($dev) + ->format("dos") + ->target("/dev/") + ->as_xml; + + diag "Defining persistent storage pool $poolxml"; + my $pool; + ok_pool(sub { $pool = $conn->define_storage_pool($poolxml) }, "define persistent storage pool"); + + # Since we blew away the partition table we should not be able to + # start the pool yet + ok_error(sub { $pool->create }, "unable to start un-built storage pool"); + + lives_ok(sub { $pool->build(0) }, "built storage pool"); + + # We should get an error if trying to build a pool which already + # has a partition table written. + ok_error(sub { $pool->build(0) }, "prevent rebuilding existing storage pool"); + + lives_ok(sub { $pool->create }, "started storage pool"); + + my @vols = $pool->list_volumes(); + + is($#vols, -1, "no storage volumes in new pool"); + + my $poolinfo = $pool->get_info(); + + ok($poolinfo->{available} > 1000, "there is some space available in the pool"); + + my $volbase = $dev; + $volbase =~ s,/dev/,,; + + my $vol1xml = $tck->generic_volume($volbase . "1", undef, 1024*1024*256)->as_xml; + my $vol2xml = $tck->generic_volume($volbase . "2", undef, 1024*1024*64)->as_xml; + + diag "Vol $vol1xml $vol2xml"; + my $vol1; + my $vol2; + ok_volume(sub { $vol1 = $pool->create_volume($vol1xml) }, "create disk partition"); + ok_volume(sub { $vol2 = $pool->create_volume($vol2xml) }, "create disk partition"); + + for my $vol (($vol1, $vol2)) { + my $path = xpath($vol, "string(/volume/target/path)"); + my $st = stat($path); + + ok($st, "path $path exists"); + } + + @vols = $pool->list_volumes(); + is($#vols, 1, "two storage volumes in pool"); + + lives_ok(sub { $vol1->delete(0) }, "deleted volume"); + + @vols = $pool->list_volumes(); + is($#vols, 0, "one storage volume in pool"); + + lives_ok(sub { $vol2->delete(0) }, "deleted volume"); + + @vols = $pool->list_volumes(); + is($#vols, -1, "zero storage volume in pool"); + + lives_ok(sub { $pool->destroy() }, "destroyed pool"); + lives_ok(sub { $pool->delete(0) }, "deleted pool"); + + # Since we blew away the partition table we should not be able to + # start the pool anymore + ok_error(sub { $pool->create }, "unable to start un-built storage pool"); + + lives_ok(sub { $pool->undefine() }, "undefined pool"); +} -- 1.6.5.2

3 3

[libvirt] Problem building on Fedora Rawhide
by Laine Stump 04 Mar '10

04 Mar '10

I just checked out a clean tree on a Fedora Rawhide system (up to date as of today), and ran "./autogen.sh && make", and ended up with the following error. Am I missing something, or is configure missing something? make[3]: Entering directory `/home/laine/devel/libvirt/daemon' CC libvirtd-event.o CC libvirtd-libvirtd.o CC libvirtd-remote.o CC libvirtd-dispatch.o CC libvirtd-stream.o CC libvirtd-remote_protocol.o CCLD libvirtd ../src/.libs/libvirt.so: undefined reference to `g_pfnGetFunctions' ../src/.libs/libvirt.so: undefined reference to `g_pVBoxFuncs' ../src/.libs/libvirt.so: undefined reference to `VBoxCGlueInit'

1 0

[libvirt] [PATCH] Fix parser checking of storage pool device
by Daniel P. Berrange 03 Mar '10

03 Mar '10

The storage backend implementations all presume that the XML parser is validating correctness of the source specification. The check for a source device was lost at some point. This allowed for a potential crash in the disk backend. Re-introduce the sanity check * src/conf/storage_conf.c: Re-add check for source device --- src/conf/storage_conf.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 19a1db9..b0f326f 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -689,6 +689,15 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) { } } + /* If DEVICE is the only source type, then its required */ + if (options->flags == VIR_STORAGE_POOL_SOURCE_DEVICE) { + if (!ret->source.ndevice) { + virStorageReportError(VIR_ERR_XML_ERROR, + "%s", _("missing storage pool source device name")); + goto cleanup; + } + } + if ((ret->target.path = virXPathString("string(./target/path)", ctxt)) == NULL) { virStorageReportError(VIR_ERR_XML_ERROR, "%s", _("missing storage pool target path")); -- 1.6.5.2

3 2

[libvirt] [PATCH] Fix mis-leading error message in pool delete API
by Daniel P. Berrange 03 Mar '10

03 Mar '10

When trying to delete a pool the error message claimed the volume could not be deleted. * src/storage/storage_driver.c: Error message referred to volumes instead of pools --- src/storage/storage_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 6b1045a..f3be6de 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -822,7 +822,7 @@ storagePoolDelete(virStoragePoolPtr obj, if (!backend->deletePool) { virStorageReportError(VIR_ERR_NO_SUPPORT, - "%s", _("pool does not support volume delete")); + "%s", _("pool does not support pool deletion")); goto cleanup; } if (backend->deletePool(obj->conn, pool, flags) < 0) -- 1.6.5.2

3 2

[libvirt] [PATCH 0/2] virsh: consistent N_() usage
by Eric Blake 03 Mar '10

03 Mar '10

As discussed here[1], we aren't being very smart about the use of gettext_noop(). In particular, most packages use N_() to mean gettext_noop rather than a synonym to _(); furthermore, our current definition of N_() meant that we were effectively calling gettext(gettext("string")), which has undefined results. Patch 1/2 also fixes a potential crash due to a translation bug, by telling xgettext to enforce that the translation of a string later handed to printf does not violate printf constraints. [1] https://www.redhat.com/archives/libvir-list/2010-March/msg00197.html [PATCH 1/2] virsh: fix existing N_ uses [PATCH 2/2] virsh: use N_ rather than gettext_noop .../0008-Step-8-of-8-Add-virsh-support.patch | 21 +- src/internal.h | 2 +- tools/virsh.c | 895 ++++++++++---------- 3 files changed, 459 insertions(+), 459 deletions(-)

1 2

[libvirt] [PATCH] Add a define for NFS_SUPER_MAGIC
by Chris Lalancette 03 Mar '10

03 Mar '10

Commit 3c12a67b766cce51b47861ccde2be41de369f832 added a dependency on the NFS_SUPER_MAGIC macro, which is defined in linux/magic.h. Unfortunately linux/magic.h is not available in RHEL-5, and causes a compile error. Just define it locally, since this is something that can't change. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/qemu/qemu_driver.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ccb9ef9..efc3be1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -49,8 +49,10 @@ #ifdef __linux__ #include <sys/vfs.h> -#include <linux/magic.h> -#endif +#ifndef NFS_SUPER_MAGIC +#define NFS_SUPER_MAGIC 0x6969 +#endif /* NFS_SUPER_MAGIC */ +#endif /* __linux__ */ #include "virterror_internal.h" #include "logging.h" -- 1.6.6.1

4 5

[libvirt] [PATCH] util: ensure safe{read, write, zero} return is checked
by Eric Blake 03 Mar '10

03 Mar '10

Based on a warning from coverity. The safe* functions guarantee complete transactions on success, but don't guarantee freedom from failure. * src/util/util.h (saferead, safewrite, safezero): Add ATTRIBUTE_RETURN_CHECK. * src/remote/remote_driver.c (remoteIO, remoteIOEventLoop): Ignore some failures. (remoteIOReadBuffer): Adjust error messages on read failure. * daemon/event.c (virEventHandleWakeup): Ignore read failure. --- Coverity claimed that safewrite was checked 37 out of 46 times, but only flagged one instance of a CHECKED_RETURN report, against remote_driver.c. I didn't spot the 9 unchecked calls that coverity's numbers would imply. Meanwhile, adding the attribute ensures that we don't forget this in the future. Please double-check if my use of ignore_value on some of the previously unchecked calls makes sense, or if we need a more invasive patch to deal with real failure. daemon/event.c | 5 +++-- src/remote/remote_driver.c | 13 +++++++++---- src/util/util.h | 9 ++++++--- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/daemon/event.c b/daemon/event.c index 2218a3e..36e9dd3 100644 --- a/daemon/event.c +++ b/daemon/event.c @@ -1,8 +1,8 @@ /* * event.c: event loop for monitoring file handles * + * Copyright (C) 2007, 2010 Red Hat, Inc. * Copyright (C) 2007 Daniel P. Berrange - * Copyright (C) 2007 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 @@ -35,6 +35,7 @@ #include "event.h" #include "memory.h" #include "util.h" +#include "ignore-value.h" #define EVENT_DEBUG(fmt, ...) DEBUG(fmt, __VA_ARGS__) @@ -630,7 +631,7 @@ static void virEventHandleWakeup(int watch ATTRIBUTE_UNUSED, { char c; virEventLock(); - saferead(fd, &c, sizeof(c)); + ignore_value (saferead(fd, &c, sizeof(c))); virEventUnlock(); } diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index def4617..47d8c56 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7809,7 +7809,9 @@ remoteIOReadBuffer(virConnectPtr conn, char errout[1024] = "\0"; if (priv->errfd != -1) { - saferead(priv->errfd, errout, sizeof(errout)); + if (saferead(priv->errfd, errout, sizeof(errout)) < 0) { + strncpy (errout, _("unknown reason"), sizeof errout); + } } virReportSystemError(errno, @@ -7818,7 +7820,9 @@ remoteIOReadBuffer(virConnectPtr conn, } else { char errout[1024] = "\0"; if (priv->errfd != -1) { - saferead(priv->errfd, errout, sizeof(errout)); + if (saferead(priv->errfd, errout, sizeof(errout)) < 0) { + strncpy (errout, _("unknown reason"), sizeof errout); + } } errorf (in_open ? NULL : conn, @@ -8431,7 +8435,8 @@ remoteIOEventLoop(virConnectPtr conn, if (fds[1].revents) { DEBUG0("Woken up from poll by other thread"); - saferead(priv->wakeupReadFD, &ignore, sizeof(ignore)); + ignore_value (saferead(priv->wakeupReadFD, &ignore, + sizeof(ignore))); } if (ret < 0) { @@ -8575,7 +8580,7 @@ remoteIO(virConnectPtr conn, priv->waitDispatch = thiscall; /* Force other thread to wakup from poll */ - safewrite(priv->wakeupSendFD, &ignore, sizeof(ignore)); + ignore_value (safewrite(priv->wakeupSendFD, &ignore, sizeof(ignore))); DEBUG("Going to sleep %d %p %p", thiscall->proc_nr, priv->waitDispatch, thiscall); /* Go to sleep while other thread is working... */ diff --git a/src/util/util.h b/src/util/util.h index cc05abe..34b77fa 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -2,6 +2,7 @@ /* * utils.h: common, generic utility functions * + * Copyright (C) 2010 Red Hat, Inc. * Copyright (C) 2006, 2007 Binary Karma * Copyright (C) 2006 Shuveb Hussain * @@ -31,9 +32,11 @@ #include <sys/select.h> #include <sys/types.h> -int saferead(int fd, void *buf, size_t count); -ssize_t safewrite(int fd, const void *buf, size_t count); -int safezero(int fd, int flags, off_t offset, off_t len); +int saferead(int fd, void *buf, size_t count) ATTRIBUTE_RETURN_CHECK; +ssize_t safewrite(int fd, const void *buf, size_t count) + ATTRIBUTE_RETURN_CHECK; +int safezero(int fd, int flags, off_t offset, off_t len) + ATTRIBUTE_RETURN_CHECK; enum { VIR_EXEC_NONE = 0, -- 1.6.6.1

1 1

[libvirt] [PATCH] Fix typo in QEMU migration command name
by Daniel P. Berrange 03 Mar '10

03 Mar '10

The QMP code was running query-migration instead of query-migrate. This doesn't work so well * src/qemu/qemu_monitor_json.c: s/query-migration/query-migrate/ --- src/qemu/qemu_monitor_json.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 3a94dd0..9c0245c 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1108,7 +1108,7 @@ int qemuMonitorJSONGetMigrationStatus(qemuMonitorPtr mon, unsigned long long *total) { int ret; - virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-migration", + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-migrate", NULL); virJSONValuePtr reply = NULL; -- 1.6.5.2

2 1

[libvirt] [PATCH] Add TCK script for host PCI device hotplug/unplug
by Daniel P. Berrange 03 Mar '10

03 Mar '10

This adds support for testing host PCI device hotplug/unplug in libvirt drivers. This requires that the host have a usable IOMMU & hardware virt, and that the device in question can be reset safely. Since the TCK can't assume there are any host PCI devices that can be safely messed around with, the person running the test suite must first list one or more devices in the config file. If no devices are listed, the test will automatically skip all parts, rather than failing. * conf/default.cfg: Config entry to specify PCI devices that the TCK can mess around with * lib/Sys/Virt/TCK.pm: API for getting a host PCI device from the config * scripts/domain/250-pci-host-hotplug.t: Test case for PCI device hotplug/unplug. --- conf/default.cfg | 15 ++++ lib/Sys/Virt/TCK.pm | 18 +++++ scripts/domain/250-pci-host-hotplug.t | 117 +++++++++++++++++++++++++++++++++ 3 files changed, 150 insertions(+), 0 deletions(-) create mode 100644 scripts/domain/250-pci-host-hotplug.t diff --git a/conf/default.cfg b/conf/default.cfg index f9c2b5c..5b85cbc 100644 --- a/conf/default.cfg +++ b/conf/default.cfg @@ -112,3 +112,18 @@ host_usb_devices = ( # } # Can list more than one USB device if many are available ) + + +# Host PCI devices that the test suite can safely mess around with +# without risk of breaking the host OS. Using a NIC device is the +# most reliable option. Definitely don't try listing a VGA device. +host_pci_devices = ( +# Slot is only required entry, all others default to 0 +# { +# domain = 0000 +# bus = 00 +# slot = 07 +# function = 0 +# } +# Can list more than one PCI device if many are available +) diff --git a/lib/Sys/Virt/TCK.pm b/lib/Sys/Virt/TCK.pm index ce03935..f101937 100644 --- a/lib/Sys/Virt/TCK.pm +++ b/lib/Sys/Virt/TCK.pm @@ -787,4 +787,22 @@ sub get_host_usb_device { return ($bus, $device, $vendor, $product); } +sub get_host_pci_device { + my $self = shift; + my $devindex = @_ ? shift : 0; + + my $devs = $self->config("host_pci_devices", []); + + if ($devindex > $#{$devs}) { + return (); + } + + my $domain = $self->config("host_pci_devices/[$devindex]/domain", 0); + my $bus = $self->config("host_pci_devices/[$devindex]/bus", 0); + my $slot = $self->config("host_pci_devices/[$devindex]/slot"); + my $function = $self->config("host_pci_devices/[$devindex]/function", 0); + + return ($domain, $bus, $slot, $function); +} + 1; diff --git a/scripts/domain/250-pci-host-hotplug.t b/scripts/domain/250-pci-host-hotplug.t new file mode 100644 index 0000000..9ccd036 --- /dev/null +++ b/scripts/domain/250-pci-host-hotplug.t @@ -0,0 +1,117 @@ +# -*- perl -*- +# +# Copyright (C) 2009 Red Hat +# Copyright (C) 2009 Daniel P. Berrange +# +# This program is free software; You can redistribute it and/or modify +# it under the GNU General Public License as published by the Free +# Software Foundation; either version 2, or (at your option) any +# later version +# +# The file "LICENSE" distributed along with this file provides full +# details of the terms and conditions +# + +=pod + +=head1 NAME + +domain/250-pci-host-hotplug.t - verify hot plug & unplug of a host PCI device + +=head1 DESCRIPTION + +The test case validates that it is possible to hotplug a pci +host device to a running domain, and then unplug it again. +This requires that the TCK configuration file have at least +one host PCI device listed. + +This first searches for the node device, then detachs it from +the host OS. Next it performs a reset. Then does the hotplug +and unplug, before finally reattaching to the host. + +=cut + +use strict; +use warnings; + +use Test::More tests => 10; + +use Sys::Virt::TCK; +use Test::Exception; + +my $tck = Sys::Virt::TCK->new(); +my $conn = eval { $tck->setup(); }; +BAIL_OUT "failed to setup test harness: $@" if $@; +END { + $tck->cleanup if $tck; +} + + +my $xml = $tck->generic_domain("tck")->as_xml; + +diag "Creating a new transient domain"; +my $dom; +ok_domain(sub { $dom = $conn->create_domain($xml) }, "created transient domain object"); + + +my ($domain, $bus, $slot, $function) = $tck->get_host_pci_device(); + +SKIP: { + # Must have one item be non-zero + unless ($domain || $bus || $slot || $function) { + skip "No host pci device available in configuration file", 9; + } + + my $nodedev; + my @devs = $conn->list_node_devices("pci"); + foreach my $dev (@devs) { + my $thisxml = $dev->get_xml_description(); + + my $xp = XML::XPath->new(xml => $thisxml); + + #warn $thisxml; + my $ndomain = $xp->find("string(/device/capability[\@type='pci']/domain[text()])")->value(); + my $nbus = $xp->find("string(/device/capability[\@type='pci']/bus[text()])")->value(); + my $nslot = $xp->find("string(/device/capability[\@type='pci']/slot[text()])")->value(); + my $nfunction = $xp->find("string(/device/capability[\@type='pci']/function[text()])")->value(); + + if ($ndomain == $domain && + $nbus == $bus && + $nslot == $slot && + $nfunction == $function) { + $nodedev = $dev; + last; + } + } + + ok(defined $nodedev, "found PCI device $domain:$bus:$slot.$function on host"); + + lives_ok(sub { $nodedev->dettach() }, "detached device from host OS"); + lives_ok(sub { $nodedev->reset() }, "reset the host PCI device"); + + my $devxml = + "<hostdev mode='subsystem' type='pci' managed='no'>\n" . + " <source>\n" . + " <address domain='$domain' bus='$bus' slot='$slot' function='$function'/>\n" . + " </source>\n" . + "</hostdev>\n"; + + my $initialxml = $dom->get_xml_description; + + diag "Attaching the new dev $devxml"; + lives_ok(sub { $dom->attach_device($devxml); }, "PCI dev has been attached"); + + my $newxml = $dom->get_xml_description; + ok($newxml =~ m|<hostdev|, "new XML has extra PCI dev present"); + + diag "Detaching the new dev $devxml"; + lives_ok(sub { $dom->detach_device($devxml); }, "PCI dev has been detached"); + + lives_ok(sub { $nodedev->reset() }, "reset the host PCI device"); + lives_ok(sub { $nodedev->reattach() }, "reattached device to host OS"); + + my $finalxml = $dom->get_xml_description; + + is($initialxml, $finalxml, "final XML has removed the disk") +} + -- 1.6.5.2

2 1

[libvirt] [PATCH] uml: avoid crash on partial read
by Eric Blake 03 Mar '10

03 Mar '10

Coverity detected a potential dereference of uninitialized memory if recvfrom got cut short. * src/uml/uml_driver.c (umlMonitorCommand): Validate complete read prior to dereferencing res. --- The patch borrows ideas from macvtap.c, the only other file in libvirt that currently uses recvfrom. I did not analyze whether this is a security hole, where a malicious UDP packet could intentionally force the dereferencing of uninitialized memory to misbehave in a controlled manner. src/uml/uml_driver.c | 14 ++++++++++++-- 1 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index bbea429..eec239f 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -733,14 +733,24 @@ static int umlMonitorCommand(virConnectPtr conn, } do { + ssize_t nbytes; addrlen = sizeof(addr); - if (recvfrom(priv->monitor, &res, sizeof res, 0, - (struct sockaddr *)&addr, &addrlen) < 0) { + nbytes = recvfrom(priv->monitor, &res, sizeof res, 0, + (struct sockaddr *)&addr, &addrlen) < 0; + if (nbytes < 0) { + if (errno == EAGAIN || errno == EINTR) + continue; virReportSystemError(errno, _("cannot read reply %s"), cmd); goto error; } + if (nbytes < sizeof res) { + virReportSystemError(errno, + _("incomplete reply %s"), + cmd); + goto error; + } if (VIR_REALLOC_N(retdata, retlen + res.length) < 0) { virReportOOMError(); -- 1.6.6.1

3 10