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@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list