[libvirt] [TCK PATCHv3] block devices: allow specification of size for safety

[Daniel P. Berrange]

On Wed, May 05, 2010 at 03:39:16PM -0600, Eric Blake wrote:
> I was getting failures of domain/103-blockdev-save-restore.t when
> connecting as qemu:///session, since my uid could stat /dev/sdb but
> not open it.  That test now skips for unprivileged users, as well as
> adds a layer of sanity checking against expected size to avoid
> trashing the wrong device.
> 
> * conf/default.cfg (host_block_devices): Document optional size.
> * lib/Sys/Virt/TCK.pm (get_host_block_device): If optional size is
> supplied, skip a device that does not match.  Also, avoid devices
> that can't be opened.
> ---
> 
> Changes from v2:
> + guarantee that the device can be opened by the current user, even if
> the .cfg file used the older path-only configuration
> 
>  conf/default.cfg    |    8 ++++++++
>  lib/Sys/Virt/TCK.pm |   17 ++++++++++++++++-
>  2 files changed, 24 insertions(+), 1 deletions(-)
> 
> diff --git a/conf/default.cfg b/conf/default.cfg
> index 01f438c..d749f5f 100644
> --- a/conf/default.cfg
> +++ b/conf/default.cfg
> @@ -134,5 +134,13 @@ host_pci_devices = (
>  # the test suite itself needs to create partitions.
>  # The disks should be at *least* 512 MB in size
>  host_block_devices = (
> +# Each block device is either a raw path
>  #  /dev/vdb
> +# or a path plus size in 1k blocks, as in /proc/partitions, to avoid
> +# trashing the wrong device
> +#  {
> +#    path = /dev/disk/by-id/usb-Generic_Flash_Disk_9B46E3C5-0:0
> +#    size = 989184
> +#  }
> +# Can list more than on block device if many are available
>  )
> diff --git a/lib/Sys/Virt/TCK.pm b/lib/Sys/Virt/TCK.pm
> index 9cdef09..0aba557 100644
> --- a/lib/Sys/Virt/TCK.pm
> +++ b/lib/Sys/Virt/TCK.pm
> @@ -33,6 +33,7 @@ use IO::Uncompress::Gunzip qw(gunzip);
>  use IO::Uncompress::Bunzip2 qw(bunzip2);
>  use XML::XPath;
>  use Carp qw(cluck carp);
> +use Fcntl qw(O_RDONLY SEEK_END);
> 
>  use Test::Builder;
>  use Sub::Uplevel qw(uplevel);
> @@ -833,7 +834,21 @@ sub get_host_block_device {
>      my $self = shift;
>      my $devindex = @_ ? shift : 0;
> 
> -    return $self->config("host_block_devices/[$devindex]", undef);
> +    my $device = $self->config("host_block_devices/[$devindex]/path", undef);
> +    my $kb_blocks = $self->config("host_block_devices/[$devindex]/size", 0);
> +    my $match = 1;
> +
> +    $device ||= $self->config("host_block_devices/[$devindex]", undef);
> +
> +    # Filter out devices that the current user can't open.
> +    sysopen(BLK, $device, O_RDONLY) or return undef;
> +
> +    if ($kb_blocks) {
> +	$match = sysseek(BLK, 0, SEEK_END) == $kb_blocks * 1024;
> +    }
> +    close BLK;
> +
> +    return $match ? $device : undef;
>  }

NB, this will result in use of a unndefined $device value if no block devices
are listed in the config. You need add

    return undef unless $device;

before the sysopen line to avoid it.

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 :|