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

[Dave Allan]

On Wed, May 05, 2010 at 08:54:35PM +0200, Jim Meyering wrote:
> 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.

Can we provide the option to specify the device serial number so that
it's really impossible to trash 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.
> > ---
> >
> > Go easy on me - I'm not that fluent in perl (yet); if there's
> > a better way to do the sanity check, I'm all ears.
> 
> The overall approach sounds fine, from my limited perspective.
> 
> > diff --git a/lib/Sys/Virt/TCK.pm b/lib/Sys/Virt/TCK.pm
> ...
> > -    return $self->config("host_block_devices/[$devindex]", undef);
> > +    my $device = $self->config("host_block_devices/[$devindex]/path", undef);
> > +    my $size = $self->config("host_block_devices/[$devindex]/size", 0);
> > +
> > +    if (!defined $device) {
> > +	$device = $self->config("host_block_devices/[$devindex]", undef);
> > +    }
> 
> This can be equivalently (idiomatically) written as:
> 
>        $device ||= $self->config("host_block_devices/[$devindex]", undef);
> 
> I prefer that, since it eliminates one use of "$device".
> 
> > +    if ($size) {
> > +	sysopen(BLK, "$device", O_RDONLY) or return undef;
> > +	return undef unless sysseek(BLK, 0, SEEK_END) == $size * 1024;
> > +	close(BLK);
> > +    }
> 
> (Note no need for double quotes around $device;
>  Leaving parens off of some built-ins like "close" is a personal
>  preference (less syntax is better), but obviously keeping in sync
>  with the prevailing style is more important)
> 
> This is similar, but doesn't leave BLK open upon failure or size mismatch:
> 
>     if ($size) {
>         my $match = (sysopen (BLK, $device, O_RDONLY)
>                      && sysseek (BLK, 0, SEEK_END) == $size * 1024);
>         close BLK;
>         $match or return undef;
>     }
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list