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

From: Fiona Ebner <f.ebner@proxmox.com>
To: Thomas Lamprecht <t.lamprecht@proxmox.com>,
	Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH v3 storage 1/9] plugin: add method to get qemu blockdevice options for volume
Date: Tue, 1 Jul 2025 13:52:45 +0200	[thread overview]
Message-ID: <00842c28-b265-434e-9b42-2874c6195399@proxmox.com> (raw)
In-Reply-To: <52125694-2521-4e90-84d8-0a4b96990820@proxmox.com>

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?

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel