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