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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]