[pve-devel] [PATCH qemu-server 02/14] blockdev: cmdline: convert drive to blockdev syntax

[Fiona Ebner]

Am 07.05.25 um 10:41 schrieb Fabian Grünbichler:
>> Fiona Ebner <f.ebner at proxmox.com> hat am 06.05.2025 14:57 CEST geschrieben:
>>> +
>>> +	($storeid) = PVE::Storage::parse_volume_id($volid, 1);
>>> +	my $vtype = $storeid ? (PVE::Storage::parse_volname($storecfg, $drive->{file}))[0] : undef;
>>> +	die "$driveid: explicit media parameter is required for iso images\n"
>>> +	    if !defined($drive->{media}) && defined($vtype) && $vtype eq 'iso';
>>> +
>>> +	if (drive_is_cdrom($drive)) {
>>> +	    $path = get_iso_path($storecfg, $volid);
>>> +	} elsif ($storeid) {
>>> +	    $path = PVE::Storage::path($storecfg, $volid, $snap);
>>> +	    $scfg = PVE::Storage::storage_config($storecfg, $storeid);
>>> +	}
>>> +    }
>>> +
>>> +    if ($path =~ m/^rbd:(\S+)$/) {
>>> +
>>> +        my $rbd_options = $1;
>>> +        $blockdev->{driver} = 'rbd';
>>> +
>>> +	#map options to key=value pair (if not key is provided, this is the image)
>>> +	#options are seprated with : but we need to exclude \: used for ipv6 address
>>> +	my $options = {
>>> +	    map {
>>> +		s/\\:/:/g; /^(.*?)=(.*)/ ? ($1=>$2) : (image=>$_)
>>> +	    } $rbd_options =~ /(?:\\:|\[[^\]]*\]|[^:\\])+/g
>>> +	};
>>
>> Maybe we should add a new method to the storage plugin API to give us a
>> hash with the necessary QEMU blockdev options? Because right now, we
>> construct an implicit QEMU-path just to deconstruct it again, which is
>> not nice at all. We can implement that for all our plugins and, for
>> backwards-compatibility handling with third-party plugins, the default
>> implementation in Plugin.pm could have the code you put here.
> 
> something like that sounds okay to me..

Okay, I'll see if I can get around to implement a draft tomorrow/next week.

>> Just not sure if the RBD plugin should put the generated ceph config in
>> /run/qemu-server/${storeid}.ceph.conf then, or use some kind of
>> /run/pve-storage path?
> 
> the latter seems like a better fit if the storage plugin generates the
> file..
> 
>> CC @Fabian, opinions?
> 
> do we need both? or could a per-volume config file also replace the hash?

Not completely, as you still need { driver => 'rbd', conf =>
'/path/to/ceph.conf', ... } and other things that cannot be part of the
ceph config, i.e. image spec? So I'm not sure we'd gain too much and
there might be other plugins requiring hashes (one example would be
gluster, although it will be dropped for PVE 9).