[pve-devel] [PATCH v3 storage 1/9] plugin: add method to get qemu blockdevice options for volume

[Fiona Ebner]

Am 01.07.25 um 13:01 schrieb Fiona Ebner:
> Am 01.07.25 um 11:28 schrieb Thomas Lamprecht:
>> Am 26.06.25 um 16:40 schrieb Fiona Ebner:
>>> +
>>> +    my $blockdev = {};
>>> +
>>> +    my ($path) = $class->filesystem_path($scfg, $volname);
>>> +
>>> +    if ($path =~ m|^/|) {
>>> +        # The 'file' driver only works for regular files. The check below is taken from
>>> +        # block/file-posix.c:hdev_probe_device() in QEMU. Do not bother with detecting 'host_cdrom'
>>> +        # devices here, those are not managed by the storage layer.
>>> +        my $st = File::stat::stat($path) or die "stat for '$path' failed - $!\n";
>>> +        my $driver = (S_ISCHR($st->mode) || S_ISBLK($st->mode)) ? 'host_device' : 'file';
>>> +        $blockdev = { driver => $driver, filename => $path };
>>> +    } else {
>>> +        die "storage plugin doesn't implement qemu_blockdev_options() method\n";
>>> +    }
>>
>> Should we rather default to an empty set of extra options? At least for external
>> plugins that would be the safer choice for upgrading, might not always work but
>> as is users can only loose FWICT?
> 
> What extra options do you mean? The default implementation here only
> sets driver and filename which is the most minimal possible.

Do you mean restricting what the individual plugins may return and
verify that in the Storage.pm's implementation of qemu_blockdev_options()?

E.g. a hash with entries for allowed drivers and allowed driver-specific
options like
$allowed = {
  file => {
    filename => 1,
  },
  host_device => {
    filename => 1,
  },
  rbd => {
     ...
  },
  ...
};

Then plugin authors that already require a custom implementation would
need to tell us what they need first and we'd need to allow it.

Is this somewhat what you meant?