[pve-devel] [PATCH v2 qemu-server 04/28] Create Drive.pm and move some drive-related code there

Fabian Grünbichler f.gruenbichler at proxmox.com
Tue Feb 25 13:30:55 CET 2020


On February 25, 2020 1:19 pm, Fabian Ebner wrote:
> On 2/25/20 12:32 PM, Fabian Grünbichler wrote:
>> this is missing some stuff (line numbers refer to after all patches have
>> been applied):
>> 
>> PVE/API2/Qemu.pm:1061:        if (PVE::QemuServer::is_valid_drivename($opt)) {
>> PVE/API2/Qemu.pm:1063:            my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt});
>> PVE/API2/Qemu.pm:1067:            $param->{$opt} = PVE::QemuServer::print_drive($drive);
>> PVE/API2/Qemu.pm:1147:                    my $drive = PVE::QemuServer::parse_drive($opt, $val);
>> PVE/API2/Qemu.pm:1160:                } elsif (PVE::QemuServer::is_valid_drivename($opt)) {
>> PVE/API2/Qemu.pm:1163:                    PVE::QemuServer::vmconfig_register_unused_drive($storecfg, $vmid, $conf, PVE::QemuServer::parse_drive($opt, $val))
>> PVE/API2/Qemu.pm:1196:                if (PVE::QemuServer::is_valid_drivename($opt)) {
>> PVE/API2/Qemu.pm:1197:                    my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt});
>> PVE/API2/Qemu.pm:1204:                    PVE::QemuServer::vmconfig_register_unused_drive($storecfg, $vmid, $conf, PVE::QemuServer::parse_drive($opt, $conf->{pending}->{$opt}))
>> PVE/API2/Qemu.pm:150:     $res->{$ds} = PVE::QemuServer::print_drive($disk);
>> PVE/API2/Qemu.pm:172:     $res->{$ds} = PVE::QemuServer::print_drive($disk);
>> PVE/API2/Qemu.pm:191:     $res->{$ds} = PVE::QemuServer::print_drive($disk);
>> PVE/API2/Qemu.pm:199:         my $olddrive = PVE::QemuServer::parse_drive($ds, $conf->{$ds});
>> PVE/API2/Qemu.pm:214:     $res->{$ds} = PVE::QemuServer::print_drive($disk);
>> PVE/API2/Qemu.pm:218:    eval { PVE::QemuServer::foreach_drive($settings, $code); };
>> PVE/API2/Qemu.pm:2832:                } elsif (PVE::QemuServer::is_valid_drivename($opt)) {
>> PVE/API2/Qemu.pm:2833:                    my $drive = PVE::QemuServer::parse_drive($opt, $value);
>> PVE/API2/Qemu.pm:2913:                        $newconf->{$opt} = PVE::QemuServer::print_drive($newdrive);
>> PVE/API2/Qemu.pm:2995:                enum => [ PVE::QemuServer::valid_drive_names() ],
>> PVE/API2/Qemu.pm:3056:            my $drive = PVE::QemuServer::parse_drive($disk, $conf->{$disk});
>> PVE/API2/Qemu.pm:3100:                    $conf->{$disk} = PVE::QemuServer::print_drive($newdrive);
>> PVE/API2/Qemu.pm:317: next if PVE::QemuServer::is_valid_drivename($opt);
>> PVE/API2/Qemu.pm:3490:                enum => [PVE::QemuServer::valid_drive_names()],
>> PVE/API2/Qemu.pm:3539:            my $drive = PVE::QemuServer::parse_drive($disk, $conf->{$disk});
>> PVE/API2/Qemu.pm:3588:            $conf->{$disk} = PVE::QemuServer::print_drive($drive);
>> PVE/API2/Qemu.pm:3990:                enum => [PVE::QemuServer::valid_drive_names()],
>> PVE/API2/Qemu.pm:526:         if (PVE::QemuServer::is_valid_drivename($opt)) {
>> PVE/API2/Qemu.pm:527:             my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt});
>> PVE/API2/Qemu.pm:531:             $param->{$opt} = PVE::QemuServer::print_drive($drive);
>> PVE/API2/Qemu.pm:65:   PVE::QemuServer::foreach_drive($settings, sub {
>> PVE/API2/Qemu.pm:98:   PVE::QemuServer::foreach_drive($conf, sub {
>> PVE/QemuConfig.pm:291:        my $drive = PVE::QemuServer::parse_drive($remove_drive, $snap->{$remove_drive});
>> PVE/QemuConfig.pm:424:    PVE::QemuServer::foreach_drive($conf, $func, @param);
>> PVE/QemuConfig.pm:70:    PVE::QemuServer::foreach_drive($conf, sub {
>> PVE/QemuMigrate.pm:452:       PVE::QemuServer::foreach_drive($conf, sub {
>> PVE/QemuMigrate.pm:456:               $conf->{$key} = PVE::QemuServer::print_drive($updated);
>> PVE/QemuMigrate.pm:511:       my $drive = PVE::QemuServer::parse_drive($target_drive, $self->{target_drive}->{$target_drive}->{drivestr});
>> PVE/QemuMigrate.pm:640:           my $current_drive = PVE::QemuServer::parse_drive($targetdrive, $conf->{$targetdrive});
>> PVE/QemuMigrate.pm:641:           my $new_drive = PVE::QemuServer::parse_drive($targetdrive, $drivestr);
>> PVE/QemuMigrate.pm:714:           my $source_drive = PVE::QemuServer::parse_drive($drive, $conf->{$drive});
>> PVE/QemuMigrate.pm:715:           my $target_drive = PVE::QemuServer::parse_drive($drive, $target->{drivestr});
>> PVE/QemuServer/Cloudinit.pm:467:    PVE::QemuServer::foreach_drive($conf, sub {
>> PVE/QemuServer/ImportDisk.pm:19:    if ($drive_name && !(PVE::QemuServer::is_valid_drivename($drive_name))) {
>> PVE/QemuServer.pm:3974:       if (PVE::QemuServer::is_valid_drivename($opt)) {
>> PVE/QemuServer.pm:3975:           my $drive = PVE::QemuServer::parse_drive($opt, $conf->{$opt});
>> PVE/QemuServer.pm:6755:    my @disks = PVE::QemuServer::valid_drive_names();
>> PVE/QemuServer.pm:6759:       my $disk = PVE::QemuServer::parse_drive($ds, $conf->{$ds});
>> PVE/VZDump/QemuServer.pm:72:    PVE::QemuServer::foreach_drive($conf, sub {
>> 
>> which should have been noticed while testing..
>> 
> 
> I'm a bit confused what you mean by missing? The commit message mentions 
> why changing the call sites isn't neccessary. If you want to, I can do 
> that for the next version, but I thought exports would be fine.

I missed the explicit export vs. regular usage, sorry. the ones in 
QemuServer.pm probably could/should be cleaned up at least, since it's 
rather confusing to have a call in a module to the same module that has 
the sub already imported into its namespace ;) there are no callers 
outside of qemu-server AFAICT, so exporting might not even be needed? 
but that could also be done once the dust has settled a bit, and we know 
whether we want to move more stuff to Drive.pm or not.

> 
>> $unuseddesc could also move to Drive.pm, especially since we have talked
>> for quite a while that we might want to include more information than
>> just the volume ID in the future.
>> 
>> other possible candidates (also maybe for a separate follow-up, since
>> these should mostly be qemu-server internal without need of
>> intra-package dependencies):
>> - pve-qm-bootdisk and its verification sub
>> - resolve_first_disk
>> - unused disk cleanup in write_vm_config
>> - drive_is_cloudinit
>> - drive_is_cdrom
>> - disksize
>> - foreach_volid (either moved as-is to Drive.pm, or as variant of
>>    foreach_volume in AbstractConfig.pm?)
>> - is_volume_in_use
>> - update_disksize
>> - update_disk_config (would need some work to avoid usage of
>>    PVE::QemuConfig)
>> 
> 
> 
> Ok, I'll take a look at those and see how easily they can be moved. I'll 
> send the refactoring as its own series next time, since it isn't 
> strictly related to the migration part of the series.
> 
>> PVE::QemuServer::Drive::print_drive and
>> PVE::QemuServer::print_drive_full do totally different stuff, it might
>> be a good idea to rename either to avoid confusion (not new in this
>> series, but we might as well when we are at it ;)). print_drive_full
>> actually generates the 'drive' string for the Qemu commandline, so maybe
>> something like drive_to_qemu_string or print_drive_qemu (as always,
>> naming is not my strength ;))?
>> 
> Ok.
> 
>> On February 24, 2020 1:43 pm, Fabian Ebner wrote:
>>> Apart from moving and adapting two places where $MAX_SATA_DISKS is used,
>>> the initialization for the drive keys in $confdesc is changed
>>> to be a single for-loop iterating over the keys of $drivedesc_hash.
>>>
>>> To avoid the need to change all the call sites, the functions are
>>> exported from the submodule and imported into QemuServer.pm.
>>>
>>> Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
>>> ---
>>>   PVE/QemuServer.pm       | 459 +--------------------------------------
>>>   PVE/QemuServer/Drive.pm | 468 ++++++++++++++++++++++++++++++++++++++++
>>>   PVE/QemuServer/Makefile |   1 +
>>>   3 files changed, 474 insertions(+), 454 deletions(-)
>>>   create mode 100644 PVE/QemuServer/Drive.pm
>>>
>>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>>> index 1580514..cf6e096 100644
>>> --- a/PVE/QemuServer.pm
>>> +++ b/PVE/QemuServer.pm
>>> @@ -44,6 +44,7 @@ use PVE::QemuConfig;
>>>   use PVE::QemuServer::Helpers qw(min_version config_aware_timeout);
>>>   use PVE::QemuServer::Cloudinit;
>>>   use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options);
>>> +use PVE::QemuServer::Drive qw(valid_drive_names is_valid_drivename parse_drive print_drive foreach_drive);
>>>   use PVE::QemuServer::Machine;
>>>   use PVE::QemuServer::Memory;
>>>   use PVE::QemuServer::Monitor qw(mon_cmd);
>>> @@ -84,13 +85,6 @@ PVE::JSONSchema::register_standard_option('pve-qm-stateuri', {
>>>       optional => 1,
>>>   });
>>>   
>>> -PVE::JSONSchema::register_standard_option('pve-qm-image-format', {
>>> -    type => 'string',
>>> -    enum => [qw(raw cow qcow qed qcow2 vmdk cloop)],
>>> -    description => "The drive's backing file's data format.",
>>> -    optional => 1,
>>> -});
>>> -
>>>   PVE::JSONSchema::register_standard_option('pve-qemu-machine', {
>>>   	description => "Specifies the Qemu machine type.",
>>>   	type => 'string',
>>> @@ -696,10 +690,6 @@ while (my ($k, $v) = each %$confdesc) {
>>>       PVE::JSONSchema::register_standard_option("pve-qm-$k", $v);
>>>   }
>>>   
>>> -my $MAX_IDE_DISKS = 4;
>>> -my $MAX_SCSI_DISKS = 31;
>>> -my $MAX_VIRTIO_DISKS = 16;
>>> -my $MAX_SATA_DISKS = 6;
>>>   my $MAX_USB_DEVICES = 5;
>>>   my $MAX_NETS = 32;
>>>   my $MAX_UNUSED_DISKS = 256;
>>> @@ -905,310 +895,6 @@ sub verify_volume_id_or_qm_path {
>>>       return $volid;
>>>   }
>>>   
>>> -my $drivedesc_hash;
>>> -
>>> -my %drivedesc_base = (
>>> -    volume => { alias => 'file' },
>>> -    file => {
>>> -	type => 'string',
>>> -	format => 'pve-volume-id-or-qm-path',
>>> -	default_key => 1,
>>> -	format_description => 'volume',
>>> -	description => "The drive's backing volume.",
>>> -    },
>>> -    media => {
>>> -	type => 'string',
>>> -	enum => [qw(cdrom disk)],
>>> -	description => "The drive's media type.",
>>> -	default => 'disk',
>>> -	optional => 1
>>> -    },
>>> -    cyls => {
>>> -	type => 'integer',
>>> -	description => "Force the drive's physical geometry to have a specific cylinder count.",
>>> -	optional => 1
>>> -    },
>>> -    heads => {
>>> -	type => 'integer',
>>> -	description => "Force the drive's physical geometry to have a specific head count.",
>>> -	optional => 1
>>> -    },
>>> -    secs => {
>>> -	type => 'integer',
>>> -	description => "Force the drive's physical geometry to have a specific sector count.",
>>> -	optional => 1
>>> -    },
>>> -    trans => {
>>> -	type => 'string',
>>> -	enum => [qw(none lba auto)],
>>> -	description => "Force disk geometry bios translation mode.",
>>> -	optional => 1,
>>> -    },
>>> -    snapshot => {
>>> -	type => 'boolean',
>>> -	description => "Controls qemu's snapshot mode feature."
>>> -	    . " If activated, changes made to the disk are temporary and will"
>>> -	    . " be discarded when the VM is shutdown.",
>>> -	optional => 1,
>>> -    },
>>> -    cache => {
>>> -	type => 'string',
>>> -	enum => [qw(none writethrough writeback unsafe directsync)],
>>> -	description => "The drive's cache mode",
>>> -	optional => 1,
>>> -    },
>>> -    format => get_standard_option('pve-qm-image-format'),
>>> -    size => {
>>> -	type => 'string',
>>> -	format => 'disk-size',
>>> -	format_description => 'DiskSize',
>>> -	description => "Disk size. This is purely informational and has no effect.",
>>> -	optional => 1,
>>> -    },
>>> -    backup => {
>>> -	type => 'boolean',
>>> -	description => "Whether the drive should be included when making backups.",
>>> -	optional => 1,
>>> -    },
>>> -    replicate => {
>>> -	type => 'boolean',
>>> -	description => 'Whether the drive should considered for replication jobs.',
>>> -	optional => 1,
>>> -	default => 1,
>>> -    },
>>> -    rerror => {
>>> -	type => 'string',
>>> -	enum => [qw(ignore report stop)],
>>> -	description => 'Read error action.',
>>> -	optional => 1,
>>> -    },
>>> -    werror => {
>>> -	type => 'string',
>>> -	enum => [qw(enospc ignore report stop)],
>>> -	description => 'Write error action.',
>>> -	optional => 1,
>>> -    },
>>> -    aio => {
>>> -	type => 'string',
>>> -	enum => [qw(native threads)],
>>> -	description => 'AIO type to use.',
>>> -	optional => 1,
>>> -    },
>>> -    discard => {
>>> -	type => 'string',
>>> -	enum => [qw(ignore on)],
>>> -	description => 'Controls whether to pass discard/trim requests to the underlying storage.',
>>> -	optional => 1,
>>> -    },
>>> -    detect_zeroes => {
>>> -	type => 'boolean',
>>> -	description => 'Controls whether to detect and try to optimize writes of zeroes.',
>>> -	optional => 1,
>>> -    },
>>> -    serial => {
>>> -	type => 'string',
>>> -	format => 'urlencoded',
>>> -	format_description => 'serial',
>>> -	maxLength => 20*3, # *3 since it's %xx url enoded
>>> -	description => "The drive's reported serial number, url-encoded, up to 20 bytes long.",
>>> -	optional => 1,
>>> -    },
>>> -    shared => {
>>> -	type => 'boolean',
>>> -	description => 'Mark this locally-managed volume as available on all nodes',
>>> -	verbose_description => "Mark this locally-managed volume as available on all nodes.\n\nWARNING: This option does not share the volume automatically, it assumes it is shared already!",
>>> -	optional => 1,
>>> -	default => 0,
>>> -    }
>>> -);
>>> -
>>> -my %iothread_fmt = ( iothread => {
>>> -	type => 'boolean',
>>> -	description => "Whether to use iothreads for this drive",
>>> -	optional => 1,
>>> -});
>>> -
>>> -my %model_fmt = (
>>> -    model => {
>>> -	type => 'string',
>>> -	format => 'urlencoded',
>>> -	format_description => 'model',
>>> -	maxLength => 40*3, # *3 since it's %xx url enoded
>>> -	description => "The drive's reported model name, url-encoded, up to 40 bytes long.",
>>> -	optional => 1,
>>> -    },
>>> -);
>>> -
>>> -my %queues_fmt = (
>>> -    queues => {
>>> -	type => 'integer',
>>> -	description => "Number of queues.",
>>> -	minimum => 2,
>>> -	optional => 1
>>> -    }
>>> -);
>>> -
>>> -my %scsiblock_fmt = (
>>> -    scsiblock => {
>>> -	type => 'boolean',
>>> -	description => "whether to use scsi-block for full passthrough of host block device\n\nWARNING: can lead to I/O errors in combination with low memory or high memory fragmentation on host",
>>> -	optional => 1,
>>> -	default => 0,
>>> -    },
>>> -);
>>> -
>>> -my %ssd_fmt = (
>>> -    ssd => {
>>> -	type => 'boolean',
>>> -	description => "Whether to expose this drive as an SSD, rather than a rotational hard disk.",
>>> -	optional => 1,
>>> -    },
>>> -);
>>> -
>>> -my %wwn_fmt = (
>>> -    wwn => {
>>> -	type => 'string',
>>> -	pattern => qr/^(0x)[0-9a-fA-F]{16}/,
>>> -	format_description => 'wwn',
>>> -	description => "The drive's worldwide name, encoded as 16 bytes hex string, prefixed by '0x'.",
>>> -	optional => 1,
>>> -    },
>>> -);
>>> -
>>> -my $add_throttle_desc = sub {
>>> -    my ($key, $type, $what, $unit, $longunit, $minimum) = @_;
>>> -    my $d = {
>>> -	type => $type,
>>> -	format_description => $unit,
>>> -	description => "Maximum $what in $longunit.",
>>> -	optional => 1,
>>> -    };
>>> -    $d->{minimum} = $minimum if defined($minimum);
>>> -    $drivedesc_base{$key} = $d;
>>> -};
>>> -# throughput: (leaky bucket)
>>> -$add_throttle_desc->('bps',     'integer', 'r/w speed',   'bps',  'bytes per second');
>>> -$add_throttle_desc->('bps_rd',  'integer', 'read speed',  'bps',  'bytes per second');
>>> -$add_throttle_desc->('bps_wr',  'integer', 'write speed', 'bps',  'bytes per second');
>>> -$add_throttle_desc->('mbps',    'number',  'r/w speed',   'mbps', 'megabytes per second');
>>> -$add_throttle_desc->('mbps_rd', 'number',  'read speed',  'mbps', 'megabytes per second');
>>> -$add_throttle_desc->('mbps_wr', 'number',  'write speed', 'mbps', 'megabytes per second');
>>> -$add_throttle_desc->('iops',    'integer', 'r/w I/O',     'iops', 'operations per second');
>>> -$add_throttle_desc->('iops_rd', 'integer', 'read I/O',    'iops', 'operations per second');
>>> -$add_throttle_desc->('iops_wr', 'integer', 'write I/O',   'iops', 'operations per second');
>>> -
>>> -# pools: (pool of IO before throttling starts taking effect)
>>> -$add_throttle_desc->('mbps_max',    'number',  'unthrottled r/w pool',       'mbps', 'megabytes per second');
>>> -$add_throttle_desc->('mbps_rd_max', 'number',  'unthrottled read pool',      'mbps', 'megabytes per second');
>>> -$add_throttle_desc->('mbps_wr_max', 'number',  'unthrottled write pool',     'mbps', 'megabytes per second');
>>> -$add_throttle_desc->('iops_max',    'integer', 'unthrottled r/w I/O pool',   'iops', 'operations per second');
>>> -$add_throttle_desc->('iops_rd_max', 'integer', 'unthrottled read I/O pool',  'iops', 'operations per second');
>>> -$add_throttle_desc->('iops_wr_max', 'integer', 'unthrottled write I/O pool', 'iops', 'operations per second');
>>> -
>>> -# burst lengths
>>> -$add_throttle_desc->('bps_max_length',     'integer', 'length of I/O bursts',       'seconds', 'seconds', 1);
>>> -$add_throttle_desc->('bps_rd_max_length',  'integer', 'length of read I/O bursts',  'seconds', 'seconds', 1);
>>> -$add_throttle_desc->('bps_wr_max_length',  'integer', 'length of write I/O bursts', 'seconds', 'seconds', 1);
>>> -$add_throttle_desc->('iops_max_length',    'integer', 'length of I/O bursts',       'seconds', 'seconds', 1);
>>> -$add_throttle_desc->('iops_rd_max_length', 'integer', 'length of read I/O bursts',  'seconds', 'seconds', 1);
>>> -$add_throttle_desc->('iops_wr_max_length', 'integer', 'length of write I/O bursts', 'seconds', 'seconds', 1);
>>> -
>>> -# legacy support
>>> -$drivedesc_base{'bps_rd_length'} = { alias => 'bps_rd_max_length' };
>>> -$drivedesc_base{'bps_wr_length'} = { alias => 'bps_wr_max_length' };
>>> -$drivedesc_base{'iops_rd_length'} = { alias => 'iops_rd_max_length' };
>>> -$drivedesc_base{'iops_wr_length'} = { alias => 'iops_wr_max_length' };
>>> -
>>> -my $ide_fmt = {
>>> -    %drivedesc_base,
>>> -    %model_fmt,
>>> -    %ssd_fmt,
>>> -    %wwn_fmt,
>>> -};
>>> -PVE::JSONSchema::register_format("pve-qm-ide", $ide_fmt);
>>> -
>>> -my $idedesc = {
>>> -    optional => 1,
>>> -    type => 'string', format => $ide_fmt,
>>> -    description => "Use volume as IDE hard disk or CD-ROM (n is 0 to " .($MAX_IDE_DISKS -1) . ").",
>>> -};
>>> -PVE::JSONSchema::register_standard_option("pve-qm-ide", $idedesc);
>>> -
>>> -my $scsi_fmt = {
>>> -    %drivedesc_base,
>>> -    %iothread_fmt,
>>> -    %queues_fmt,
>>> -    %scsiblock_fmt,
>>> -    %ssd_fmt,
>>> -    %wwn_fmt,
>>> -};
>>> -my $scsidesc = {
>>> -    optional => 1,
>>> -    type => 'string', format => $scsi_fmt,
>>> -    description => "Use volume as SCSI hard disk or CD-ROM (n is 0 to " . ($MAX_SCSI_DISKS - 1) . ").",
>>> -};
>>> -PVE::JSONSchema::register_standard_option("pve-qm-scsi", $scsidesc);
>>> -
>>> -my $sata_fmt = {
>>> -    %drivedesc_base,
>>> -    %ssd_fmt,
>>> -    %wwn_fmt,
>>> -};
>>> -my $satadesc = {
>>> -    optional => 1,
>>> -    type => 'string', format => $sata_fmt,
>>> -    description => "Use volume as SATA hard disk or CD-ROM (n is 0 to " . ($MAX_SATA_DISKS - 1). ").",
>>> -};
>>> -PVE::JSONSchema::register_standard_option("pve-qm-sata", $satadesc);
>>> -
>>> -my $virtio_fmt = {
>>> -    %drivedesc_base,
>>> -    %iothread_fmt,
>>> -};
>>> -my $virtiodesc = {
>>> -    optional => 1,
>>> -    type => 'string', format => $virtio_fmt,
>>> -    description => "Use volume as VIRTIO hard disk (n is 0 to " . ($MAX_VIRTIO_DISKS - 1) . ").",
>>> -};
>>> -PVE::JSONSchema::register_standard_option("pve-qm-virtio", $virtiodesc);
>>> -
>>> -my $alldrive_fmt = {
>>> -    %drivedesc_base,
>>> -    %iothread_fmt,
>>> -    %model_fmt,
>>> -    %queues_fmt,
>>> -    %scsiblock_fmt,
>>> -    %ssd_fmt,
>>> -    %wwn_fmt,
>>> -};
>>> -
>>> -my $efidisk_fmt = {
>>> -    volume => { alias => 'file' },
>>> -    file => {
>>> -	type => 'string',
>>> -	format => 'pve-volume-id-or-qm-path',
>>> -	default_key => 1,
>>> -	format_description => 'volume',
>>> -	description => "The drive's backing volume.",
>>> -    },
>>> -    format => get_standard_option('pve-qm-image-format'),
>>> -    size => {
>>> -	type => 'string',
>>> -	format => 'disk-size',
>>> -	format_description => 'DiskSize',
>>> -	description => "Disk size. This is purely informational and has no effect.",
>>> -	optional => 1,
>>> -    },
>>> -};
>>> -
>>> -my $efidisk_desc = {
>>> -    optional => 1,
>>> -    type => 'string', format => $efidisk_fmt,
>>> -    description => "Configure a Disk for storing EFI vars",
>>> -};
>>> -
>>> -PVE::JSONSchema::register_standard_option("pve-qm-efidisk", $efidisk_desc);
>>> -
>>>   my $usb_fmt = {
>>>       host => {
>>>   	default_key => 1,
>>> @@ -1355,29 +1041,10 @@ for (my $i = 0; $i < $MAX_HOSTPCI_DEVICES; $i++)  {
>>>       $confdesc->{"hostpci$i"} = $hostpcidesc;
>>>   }
>>>   
>>> -for (my $i = 0; $i < $MAX_IDE_DISKS; $i++)  {
>>> -    $drivedesc_hash->{"ide$i"} = $idedesc;
>>> -    $confdesc->{"ide$i"} = $idedesc;
>>> -}
>>> -
>>> -for (my $i = 0; $i < $MAX_SATA_DISKS; $i++)  {
>>> -    $drivedesc_hash->{"sata$i"} = $satadesc;
>>> -    $confdesc->{"sata$i"} = $satadesc;
>>> -}
>>> -
>>> -for (my $i = 0; $i < $MAX_SCSI_DISKS; $i++)  {
>>> -    $drivedesc_hash->{"scsi$i"} = $scsidesc;
>>> -    $confdesc->{"scsi$i"} = $scsidesc ;
>>> +for my $key (keys %{$PVE::QemuServer::Drive::drivedesc_hash}) {
>>> +    $confdesc->{$key} = $PVE::QemuServer::Drive::drivedesc_hash->{$key};
>>>   }
>>>   
>>> -for (my $i = 0; $i < $MAX_VIRTIO_DISKS; $i++)  {
>>> -    $drivedesc_hash->{"virtio$i"} = $virtiodesc;
>>> -    $confdesc->{"virtio$i"} = $virtiodesc;
>>> -}
>>> -
>>> -$drivedesc_hash->{efidisk0} = $efidisk_desc;
>>> -$confdesc->{efidisk0} = $efidisk_desc;
>>> -
>>>   for (my $i = 0; $i < $MAX_USB_DEVICES; $i++)  {
>>>       $confdesc->{"usb$i"} = $usbdesc;
>>>   }
>>> @@ -1440,21 +1107,6 @@ sub kernel_has_vhost_net {
>>>       return -c '/dev/vhost-net';
>>>   }
>>>   
>>> -sub valid_drive_names {
>>> -    # order is important - used to autoselect boot disk
>>> -    return ((map { "ide$_" } (0 .. ($MAX_IDE_DISKS - 1))),
>>> -            (map { "scsi$_" } (0 .. ($MAX_SCSI_DISKS - 1))),
>>> -            (map { "virtio$_" } (0 .. ($MAX_VIRTIO_DISKS - 1))),
>>> -            (map { "sata$_" } (0 .. ($MAX_SATA_DISKS - 1))),
>>> -            'efidisk0');
>>> -}
>>> -
>>> -sub is_valid_drivename {
>>> -    my $dev = shift;
>>> -
>>> -    return defined($drivedesc_hash->{$dev});
>>> -}
>>> -
>>>   sub option_exists {
>>>       my $key = shift;
>>>       return defined($confdesc->{$key});
>>> @@ -1571,94 +1223,6 @@ sub pve_verify_hotplug_features {
>>>       die "unable to parse hotplug option\n";
>>>   }
>>>   
>>> -# ideX = [volume=]volume-id[,media=d][,cyls=c,heads=h,secs=s[,trans=t]]
>>> -#        [,snapshot=on|off][,cache=on|off][,format=f][,backup=yes|no]
>>> -#        [,rerror=ignore|report|stop][,werror=enospc|ignore|report|stop]
>>> -#        [,aio=native|threads][,discard=ignore|on][,detect_zeroes=on|off]
>>> -#        [,iothread=on][,serial=serial][,model=model]
>>> -
>>> -sub parse_drive {
>>> -    my ($key, $data) = @_;
>>> -
>>> -    my ($interface, $index);
>>> -
>>> -    if ($key =~ m/^([^\d]+)(\d+)$/) {
>>> -	$interface = $1;
>>> -	$index = $2;
>>> -    } else {
>>> -	return undef;
>>> -    }
>>> -
>>> -    my $desc = $key =~ /^unused\d+$/ ? $alldrive_fmt
>>> -                                     : $drivedesc_hash->{$key}->{format};
>>> -    if (!$desc) {
>>> -	warn "invalid drive key: $key\n";
>>> -	return undef;
>>> -    }
>>> -    my $res = eval { PVE::JSONSchema::parse_property_string($desc, $data) };
>>> -    return undef if !$res;
>>> -    $res->{interface} = $interface;
>>> -    $res->{index} = $index;
>>> -
>>> -    my $error = 0;
>>> -    foreach my $opt (qw(bps bps_rd bps_wr)) {
>>> -	if (my $bps = defined(delete $res->{$opt})) {
>>> -	    if (defined($res->{"m$opt"})) {
>>> -		warn "both $opt and m$opt specified\n";
>>> -		++$error;
>>> -		next;
>>> -	    }
>>> -	    $res->{"m$opt"} = sprintf("%.3f", $bps / (1024*1024.0));
>>> -	}
>>> -    }
>>> -
>>> -    # can't use the schema's 'requires' because of the mbps* => bps* "transforming aliases"
>>> -    for my $requirement (
>>> -	[mbps_max => 'mbps'],
>>> -	[mbps_rd_max => 'mbps_rd'],
>>> -	[mbps_wr_max => 'mbps_wr'],
>>> -	[miops_max => 'miops'],
>>> -	[miops_rd_max => 'miops_rd'],
>>> -	[miops_wr_max => 'miops_wr'],
>>> -	[bps_max_length => 'mbps_max'],
>>> -	[bps_rd_max_length => 'mbps_rd_max'],
>>> -	[bps_wr_max_length => 'mbps_wr_max'],
>>> -	[iops_max_length => 'iops_max'],
>>> -	[iops_rd_max_length => 'iops_rd_max'],
>>> -	[iops_wr_max_length => 'iops_wr_max']) {
>>> -	my ($option, $requires) = @$requirement;
>>> -	if ($res->{$option} && !$res->{$requires}) {
>>> -	    warn "$option requires $requires\n";
>>> -	    ++$error;
>>> -	}
>>> -    }
>>> -
>>> -    return undef if $error;
>>> -
>>> -    return undef if $res->{mbps_rd} && $res->{mbps};
>>> -    return undef if $res->{mbps_wr} && $res->{mbps};
>>> -    return undef if $res->{iops_rd} && $res->{iops};
>>> -    return undef if $res->{iops_wr} && $res->{iops};
>>> -
>>> -    if ($res->{media} && ($res->{media} eq 'cdrom')) {
>>> -	return undef if $res->{snapshot} || $res->{trans} || $res->{format};
>>> -	return undef if $res->{heads} || $res->{secs} || $res->{cyls};
>>> -	return undef if $res->{interface} eq 'virtio';
>>> -    }
>>> -
>>> -    if (my $size = $res->{size}) {
>>> -	return undef if !defined($res->{size} = PVE::JSONSchema::parse_size($size));
>>> -    }
>>> -
>>> -    return $res;
>>> -}
>>> -
>>> -sub print_drive {
>>> -    my ($drive) = @_;
>>> -    my $skip = [ 'index', 'interface' ];
>>> -    return PVE::JSONSchema::print_property_string($drive, $alldrive_fmt, $skip);
>>> -}
>>> -
>>>   sub scsi_inquiry {
>>>       my($fh, $noerr) = @_;
>>>   
>>> @@ -1796,7 +1360,7 @@ sub print_drivedevice_full {
>>>   	$device .= ",wwn=$drive->{wwn}" if $drive->{wwn};
>>>   
>>>       } elsif ($drive->{interface} eq 'ide' || $drive->{interface} eq 'sata') {
>>> -	my $maxdev = ($drive->{interface} eq 'sata') ? $MAX_SATA_DISKS : 2;
>>> +	my $maxdev = ($drive->{interface} eq 'sata') ? $PVE::QemuServer::Drive::MAX_SATA_DISKS : 2;
>>>   	my $controller = int($drive->{index} / $maxdev);
>>>   	my $unit = $drive->{index} % $maxdev;
>>>   	my $devicetype = ($drive->{media} && $drive->{media} eq 'cdrom') ? "cd" : "hd";
>>> @@ -3125,19 +2689,6 @@ sub vmstatus {
>>>       return $res;
>>>   }
>>>   
>>> -sub foreach_drive {
>>> -    my ($conf, $func, @param) = @_;
>>> -
>>> -    foreach my $ds (valid_drive_names()) {
>>> -	next if !defined($conf->{$ds});
>>> -
>>> -	my $drive = parse_drive($ds, $conf->{$ds});
>>> -	next if !$drive;
>>> -
>>> -	&$func($ds, $drive, @param);
>>> -    }
>>> -}
>>> -
>>>   sub foreach_volid {
>>>       my ($conf, $func, @param) = @_;
>>>   
>>> @@ -3950,7 +3501,7 @@ sub config_to_command {
>>>           }
>>>   
>>>           if ($drive->{interface} eq 'sata') {
>>> -           my $controller = int($drive->{index} / $MAX_SATA_DISKS);
>>> +           my $controller = int($drive->{index} / $PVE::QemuServer::Drive::MAX_SATA_DISKS);
>>>              $pciaddr = print_pci_addr("ahci$controller", $bridges, $arch, $machine_type);
>>>              push @$devices, '-device', "ahci,id=ahci$controller,multifunction=on$pciaddr" if !$ahcicontroller->{$controller};
>>>              $ahcicontroller->{$controller}=1;
>>> diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
>>> new file mode 100644
>>> index 0000000..35ac99d
>>> --- /dev/null
>>> +++ b/PVE/QemuServer/Drive.pm
>>> @@ -0,0 +1,468 @@
>>> +package PVE::QemuServer::Drive;
>>> +
>>> +use strict;
>>> +use warnings;
>>> +
>>> +use PVE::JSONSchema qw(get_standard_option);
>>> +
>>> +use base qw(Exporter);
>>> +
>>> +our @EXPORT_OK = qw(
>>> +valid_drive_names
>>> +is_valid_drivename
>>> +parse_drive
>>> +print_drive
>>> +foreach_drive
>>> +);
>>> +
>>> +PVE::JSONSchema::register_standard_option('pve-qm-image-format', {
>>> +    type => 'string',
>>> +    enum => [qw(raw cow qcow qed qcow2 vmdk cloop)],
>>> +    description => "The drive's backing file's data format.",
>>> +    optional => 1,
>>> +});
>>> +
>>> +our $MAX_IDE_DISKS = 4;
>>> +our $MAX_SCSI_DISKS = 31;
>>> +our $MAX_VIRTIO_DISKS = 16;
>>> +our $MAX_SATA_DISKS = 6;
>>> +
>>> +our $drivedesc_hash;
>>> +
>>> +my %drivedesc_base = (
>>> +    volume => { alias => 'file' },
>>> +    file => {
>>> +	type => 'string',
>>> +	format => 'pve-volume-id-or-qm-path',
>>> +	default_key => 1,
>>> +	format_description => 'volume',
>>> +	description => "The drive's backing volume.",
>>> +    },
>>> +    media => {
>>> +	type => 'string',
>>> +	enum => [qw(cdrom disk)],
>>> +	description => "The drive's media type.",
>>> +	default => 'disk',
>>> +	optional => 1
>>> +    },
>>> +    cyls => {
>>> +	type => 'integer',
>>> +	description => "Force the drive's physical geometry to have a specific cylinder count.",
>>> +	optional => 1
>>> +    },
>>> +    heads => {
>>> +	type => 'integer',
>>> +	description => "Force the drive's physical geometry to have a specific head count.",
>>> +	optional => 1
>>> +    },
>>> +    secs => {
>>> +	type => 'integer',
>>> +	description => "Force the drive's physical geometry to have a specific sector count.",
>>> +	optional => 1
>>> +    },
>>> +    trans => {
>>> +	type => 'string',
>>> +	enum => [qw(none lba auto)],
>>> +	description => "Force disk geometry bios translation mode.",
>>> +	optional => 1,
>>> +    },
>>> +    snapshot => {
>>> +	type => 'boolean',
>>> +	description => "Controls qemu's snapshot mode feature."
>>> +	    . " If activated, changes made to the disk are temporary and will"
>>> +	    . " be discarded when the VM is shutdown.",
>>> +	optional => 1,
>>> +    },
>>> +    cache => {
>>> +	type => 'string',
>>> +	enum => [qw(none writethrough writeback unsafe directsync)],
>>> +	description => "The drive's cache mode",
>>> +	optional => 1,
>>> +    },
>>> +    format => get_standard_option('pve-qm-image-format'),
>>> +    size => {
>>> +	type => 'string',
>>> +	format => 'disk-size',
>>> +	format_description => 'DiskSize',
>>> +	description => "Disk size. This is purely informational and has no effect.",
>>> +	optional => 1,
>>> +    },
>>> +    backup => {
>>> +	type => 'boolean',
>>> +	description => "Whether the drive should be included when making backups.",
>>> +	optional => 1,
>>> +    },
>>> +    replicate => {
>>> +	type => 'boolean',
>>> +	description => 'Whether the drive should considered for replication jobs.',
>>> +	optional => 1,
>>> +	default => 1,
>>> +    },
>>> +    rerror => {
>>> +	type => 'string',
>>> +	enum => [qw(ignore report stop)],
>>> +	description => 'Read error action.',
>>> +	optional => 1,
>>> +    },
>>> +    werror => {
>>> +	type => 'string',
>>> +	enum => [qw(enospc ignore report stop)],
>>> +	description => 'Write error action.',
>>> +	optional => 1,
>>> +    },
>>> +    aio => {
>>> +	type => 'string',
>>> +	enum => [qw(native threads)],
>>> +	description => 'AIO type to use.',
>>> +	optional => 1,
>>> +    },
>>> +    discard => {
>>> +	type => 'string',
>>> +	enum => [qw(ignore on)],
>>> +	description => 'Controls whether to pass discard/trim requests to the underlying storage.',
>>> +	optional => 1,
>>> +    },
>>> +    detect_zeroes => {
>>> +	type => 'boolean',
>>> +	description => 'Controls whether to detect and try to optimize writes of zeroes.',
>>> +	optional => 1,
>>> +    },
>>> +    serial => {
>>> +	type => 'string',
>>> +	format => 'urlencoded',
>>> +	format_description => 'serial',
>>> +	maxLength => 20*3, # *3 since it's %xx url enoded
>>> +	description => "The drive's reported serial number, url-encoded, up to 20 bytes long.",
>>> +	optional => 1,
>>> +    },
>>> +    shared => {
>>> +	type => 'boolean',
>>> +	description => 'Mark this locally-managed volume as available on all nodes',
>>> +	verbose_description => "Mark this locally-managed volume as available on all nodes.\n\nWARNING: This option does not share the volume automatically, it assumes it is shared already!",
>>> +	optional => 1,
>>> +	default => 0,
>>> +    }
>>> +);
>>> +
>>> +my %iothread_fmt = ( iothread => {
>>> +	type => 'boolean',
>>> +	description => "Whether to use iothreads for this drive",
>>> +	optional => 1,
>>> +});
>>> +
>>> +my %model_fmt = (
>>> +    model => {
>>> +	type => 'string',
>>> +	format => 'urlencoded',
>>> +	format_description => 'model',
>>> +	maxLength => 40*3, # *3 since it's %xx url enoded
>>> +	description => "The drive's reported model name, url-encoded, up to 40 bytes long.",
>>> +	optional => 1,
>>> +    },
>>> +);
>>> +
>>> +my %queues_fmt = (
>>> +    queues => {
>>> +	type => 'integer',
>>> +	description => "Number of queues.",
>>> +	minimum => 2,
>>> +	optional => 1
>>> +    }
>>> +);
>>> +
>>> +my %scsiblock_fmt = (
>>> +    scsiblock => {
>>> +	type => 'boolean',
>>> +	description => "whether to use scsi-block for full passthrough of host block device\n\nWARNING: can lead to I/O errors in combination with low memory or high memory fragmentation on host",
>>> +	optional => 1,
>>> +	default => 0,
>>> +    },
>>> +);
>>> +
>>> +my %ssd_fmt = (
>>> +    ssd => {
>>> +	type => 'boolean',
>>> +	description => "Whether to expose this drive as an SSD, rather than a rotational hard disk.",
>>> +	optional => 1,
>>> +    },
>>> +);
>>> +
>>> +my %wwn_fmt = (
>>> +    wwn => {
>>> +	type => 'string',
>>> +	pattern => qr/^(0x)[0-9a-fA-F]{16}/,
>>> +	format_description => 'wwn',
>>> +	description => "The drive's worldwide name, encoded as 16 bytes hex string, prefixed by '0x'.",
>>> +	optional => 1,
>>> +    },
>>> +);
>>> +
>>> +my $add_throttle_desc = sub {
>>> +    my ($key, $type, $what, $unit, $longunit, $minimum) = @_;
>>> +    my $d = {
>>> +	type => $type,
>>> +	format_description => $unit,
>>> +	description => "Maximum $what in $longunit.",
>>> +	optional => 1,
>>> +    };
>>> +    $d->{minimum} = $minimum if defined($minimum);
>>> +    $drivedesc_base{$key} = $d;
>>> +};
>>> +# throughput: (leaky bucket)
>>> +$add_throttle_desc->('bps',     'integer', 'r/w speed',   'bps',  'bytes per second');
>>> +$add_throttle_desc->('bps_rd',  'integer', 'read speed',  'bps',  'bytes per second');
>>> +$add_throttle_desc->('bps_wr',  'integer', 'write speed', 'bps',  'bytes per second');
>>> +$add_throttle_desc->('mbps',    'number',  'r/w speed',   'mbps', 'megabytes per second');
>>> +$add_throttle_desc->('mbps_rd', 'number',  'read speed',  'mbps', 'megabytes per second');
>>> +$add_throttle_desc->('mbps_wr', 'number',  'write speed', 'mbps', 'megabytes per second');
>>> +$add_throttle_desc->('iops',    'integer', 'r/w I/O',     'iops', 'operations per second');
>>> +$add_throttle_desc->('iops_rd', 'integer', 'read I/O',    'iops', 'operations per second');
>>> +$add_throttle_desc->('iops_wr', 'integer', 'write I/O',   'iops', 'operations per second');
>>> +
>>> +# pools: (pool of IO before throttling starts taking effect)
>>> +$add_throttle_desc->('mbps_max',    'number',  'unthrottled r/w pool',       'mbps', 'megabytes per second');
>>> +$add_throttle_desc->('mbps_rd_max', 'number',  'unthrottled read pool',      'mbps', 'megabytes per second');
>>> +$add_throttle_desc->('mbps_wr_max', 'number',  'unthrottled write pool',     'mbps', 'megabytes per second');
>>> +$add_throttle_desc->('iops_max',    'integer', 'unthrottled r/w I/O pool',   'iops', 'operations per second');
>>> +$add_throttle_desc->('iops_rd_max', 'integer', 'unthrottled read I/O pool',  'iops', 'operations per second');
>>> +$add_throttle_desc->('iops_wr_max', 'integer', 'unthrottled write I/O pool', 'iops', 'operations per second');
>>> +
>>> +# burst lengths
>>> +$add_throttle_desc->('bps_max_length',     'integer', 'length of I/O bursts',       'seconds', 'seconds', 1);
>>> +$add_throttle_desc->('bps_rd_max_length',  'integer', 'length of read I/O bursts',  'seconds', 'seconds', 1);
>>> +$add_throttle_desc->('bps_wr_max_length',  'integer', 'length of write I/O bursts', 'seconds', 'seconds', 1);
>>> +$add_throttle_desc->('iops_max_length',    'integer', 'length of I/O bursts',       'seconds', 'seconds', 1);
>>> +$add_throttle_desc->('iops_rd_max_length', 'integer', 'length of read I/O bursts',  'seconds', 'seconds', 1);
>>> +$add_throttle_desc->('iops_wr_max_length', 'integer', 'length of write I/O bursts', 'seconds', 'seconds', 1);
>>> +
>>> +# legacy support
>>> +$drivedesc_base{'bps_rd_length'} = { alias => 'bps_rd_max_length' };
>>> +$drivedesc_base{'bps_wr_length'} = { alias => 'bps_wr_max_length' };
>>> +$drivedesc_base{'iops_rd_length'} = { alias => 'iops_rd_max_length' };
>>> +$drivedesc_base{'iops_wr_length'} = { alias => 'iops_wr_max_length' };
>>> +
>>> +my $ide_fmt = {
>>> +    %drivedesc_base,
>>> +    %model_fmt,
>>> +    %ssd_fmt,
>>> +    %wwn_fmt,
>>> +};
>>> +PVE::JSONSchema::register_format("pve-qm-ide", $ide_fmt);
>>> +
>>> +my $idedesc = {
>>> +    optional => 1,
>>> +    type => 'string', format => $ide_fmt,
>>> +    description => "Use volume as IDE hard disk or CD-ROM (n is 0 to " .($MAX_IDE_DISKS -1) . ").",
>>> +};
>>> +PVE::JSONSchema::register_standard_option("pve-qm-ide", $idedesc);
>>> +
>>> +my $scsi_fmt = {
>>> +    %drivedesc_base,
>>> +    %iothread_fmt,
>>> +    %queues_fmt,
>>> +    %scsiblock_fmt,
>>> +    %ssd_fmt,
>>> +    %wwn_fmt,
>>> +};
>>> +my $scsidesc = {
>>> +    optional => 1,
>>> +    type => 'string', format => $scsi_fmt,
>>> +    description => "Use volume as SCSI hard disk or CD-ROM (n is 0 to " . ($MAX_SCSI_DISKS - 1) . ").",
>>> +};
>>> +PVE::JSONSchema::register_standard_option("pve-qm-scsi", $scsidesc);
>>> +
>>> +my $sata_fmt = {
>>> +    %drivedesc_base,
>>> +    %ssd_fmt,
>>> +    %wwn_fmt,
>>> +};
>>> +my $satadesc = {
>>> +    optional => 1,
>>> +    type => 'string', format => $sata_fmt,
>>> +    description => "Use volume as SATA hard disk or CD-ROM (n is 0 to " . ($MAX_SATA_DISKS - 1). ").",
>>> +};
>>> +PVE::JSONSchema::register_standard_option("pve-qm-sata", $satadesc);
>>> +
>>> +my $virtio_fmt = {
>>> +    %drivedesc_base,
>>> +    %iothread_fmt,
>>> +};
>>> +my $virtiodesc = {
>>> +    optional => 1,
>>> +    type => 'string', format => $virtio_fmt,
>>> +    description => "Use volume as VIRTIO hard disk (n is 0 to " . ($MAX_VIRTIO_DISKS - 1) . ").",
>>> +};
>>> +PVE::JSONSchema::register_standard_option("pve-qm-virtio", $virtiodesc);
>>> +
>>> +my $alldrive_fmt = {
>>> +    %drivedesc_base,
>>> +    %iothread_fmt,
>>> +    %model_fmt,
>>> +    %queues_fmt,
>>> +    %scsiblock_fmt,
>>> +    %ssd_fmt,
>>> +    %wwn_fmt,
>>> +};
>>> +
>>> +my $efidisk_fmt = {
>>> +    volume => { alias => 'file' },
>>> +    file => {
>>> +	type => 'string',
>>> +	format => 'pve-volume-id-or-qm-path',
>>> +	default_key => 1,
>>> +	format_description => 'volume',
>>> +	description => "The drive's backing volume.",
>>> +    },
>>> +    format => get_standard_option('pve-qm-image-format'),
>>> +    size => {
>>> +	type => 'string',
>>> +	format => 'disk-size',
>>> +	format_description => 'DiskSize',
>>> +	description => "Disk size. This is purely informational and has no effect.",
>>> +	optional => 1,
>>> +    },
>>> +};
>>> +
>>> +my $efidisk_desc = {
>>> +    optional => 1,
>>> +    type => 'string', format => $efidisk_fmt,
>>> +    description => "Configure a Disk for storing EFI vars",
>>> +};
>>> +
>>> +PVE::JSONSchema::register_standard_option("pve-qm-efidisk", $efidisk_desc);
>>> +
>>> +for (my $i = 0; $i < $MAX_IDE_DISKS; $i++)  {
>>> +    $drivedesc_hash->{"ide$i"} = $idedesc;
>>> +}
>>> +
>>> +for (my $i = 0; $i < $MAX_SATA_DISKS; $i++)  {
>>> +    $drivedesc_hash->{"sata$i"} = $satadesc;
>>> +}
>>> +
>>> +for (my $i = 0; $i < $MAX_SCSI_DISKS; $i++)  {
>>> +    $drivedesc_hash->{"scsi$i"} = $scsidesc;
>>> +}
>>> +
>>> +for (my $i = 0; $i < $MAX_VIRTIO_DISKS; $i++)  {
>>> +    $drivedesc_hash->{"virtio$i"} = $virtiodesc;
>>> +}
>>> +
>>> +$drivedesc_hash->{efidisk0} = $efidisk_desc;
>>> +
>>> +sub valid_drive_names {
>>> +    # order is important - used to autoselect boot disk
>>> +    return ((map { "ide$_" } (0 .. ($MAX_IDE_DISKS - 1))),
>>> +            (map { "scsi$_" } (0 .. ($MAX_SCSI_DISKS - 1))),
>>> +            (map { "virtio$_" } (0 .. ($MAX_VIRTIO_DISKS - 1))),
>>> +            (map { "sata$_" } (0 .. ($MAX_SATA_DISKS - 1))),
>>> +            'efidisk0');
>>> +}
>>> +
>>> +sub is_valid_drivename {
>>> +    my $dev = shift;
>>> +
>>> +    return defined($drivedesc_hash->{$dev});
>>> +}
>>> +
>>> +# ideX = [volume=]volume-id[,media=d][,cyls=c,heads=h,secs=s[,trans=t]]
>>> +#        [,snapshot=on|off][,cache=on|off][,format=f][,backup=yes|no]
>>> +#        [,rerror=ignore|report|stop][,werror=enospc|ignore|report|stop]
>>> +#        [,aio=native|threads][,discard=ignore|on][,detect_zeroes=on|off]
>>> +#        [,iothread=on][,serial=serial][,model=model]
>>> +
>>> +sub parse_drive {
>>> +    my ($key, $data) = @_;
>>> +
>>> +    my ($interface, $index);
>>> +
>>> +    if ($key =~ m/^([^\d]+)(\d+)$/) {
>>> +	$interface = $1;
>>> +	$index = $2;
>>> +    } else {
>>> +	return undef;
>>> +    }
>>> +
>>> +    my $desc = $key =~ /^unused\d+$/ ? $alldrive_fmt
>>> +                                     : $drivedesc_hash->{$key}->{format};
>>> +    if (!$desc) {
>>> +	warn "invalid drive key: $key\n";
>>> +	return undef;
>>> +    }
>>> +    my $res = eval { PVE::JSONSchema::parse_property_string($desc, $data) };
>>> +    return undef if !$res;
>>> +    $res->{interface} = $interface;
>>> +    $res->{index} = $index;
>>> +
>>> +    my $error = 0;
>>> +    foreach my $opt (qw(bps bps_rd bps_wr)) {
>>> +	if (my $bps = defined(delete $res->{$opt})) {
>>> +	    if (defined($res->{"m$opt"})) {
>>> +		warn "both $opt and m$opt specified\n";
>>> +		++$error;
>>> +		next;
>>> +	    }
>>> +	    $res->{"m$opt"} = sprintf("%.3f", $bps / (1024*1024.0));
>>> +	}
>>> +    }
>>> +
>>> +    # can't use the schema's 'requires' because of the mbps* => bps* "transforming aliases"
>>> +    for my $requirement (
>>> +	[mbps_max => 'mbps'],
>>> +	[mbps_rd_max => 'mbps_rd'],
>>> +	[mbps_wr_max => 'mbps_wr'],
>>> +	[miops_max => 'miops'],
>>> +	[miops_rd_max => 'miops_rd'],
>>> +	[miops_wr_max => 'miops_wr'],
>>> +	[bps_max_length => 'mbps_max'],
>>> +	[bps_rd_max_length => 'mbps_rd_max'],
>>> +	[bps_wr_max_length => 'mbps_wr_max'],
>>> +	[iops_max_length => 'iops_max'],
>>> +	[iops_rd_max_length => 'iops_rd_max'],
>>> +	[iops_wr_max_length => 'iops_wr_max']) {
>>> +	my ($option, $requires) = @$requirement;
>>> +	if ($res->{$option} && !$res->{$requires}) {
>>> +	    warn "$option requires $requires\n";
>>> +	    ++$error;
>>> +	}
>>> +    }
>>> +
>>> +    return undef if $error;
>>> +
>>> +    return undef if $res->{mbps_rd} && $res->{mbps};
>>> +    return undef if $res->{mbps_wr} && $res->{mbps};
>>> +    return undef if $res->{iops_rd} && $res->{iops};
>>> +    return undef if $res->{iops_wr} && $res->{iops};
>>> +
>>> +    if ($res->{media} && ($res->{media} eq 'cdrom')) {
>>> +	return undef if $res->{snapshot} || $res->{trans} || $res->{format};
>>> +	return undef if $res->{heads} || $res->{secs} || $res->{cyls};
>>> +	return undef if $res->{interface} eq 'virtio';
>>> +    }
>>> +
>>> +    if (my $size = $res->{size}) {
>>> +	return undef if !defined($res->{size} = PVE::JSONSchema::parse_size($size));
>>> +    }
>>> +
>>> +    return $res;
>>> +}
>>> +
>>> +sub print_drive {
>>> +    my ($drive) = @_;
>>> +    my $skip = [ 'index', 'interface' ];
>>> +    return PVE::JSONSchema::print_property_string($drive, $alldrive_fmt, $skip);
>>> +}
>>> +
>>> +sub foreach_drive {
>>> +    my ($conf, $func, @param) = @_;
>>> +
>>> +    foreach my $ds (valid_drive_names()) {
>>> +	next if !defined($conf->{$ds});
>>> +
>>> +	my $drive = parse_drive($ds, $conf->{$ds});
>>> +	next if !$drive;
>>> +
>>> +	&$func($ds, $drive, @param);
>>> +    }
>>> +}
>>> +
>>> +1;
>>> diff --git a/PVE/QemuServer/Makefile b/PVE/QemuServer/Makefile
>>> index 6a49626..fd8cfbb 100644
>>> --- a/PVE/QemuServer/Makefile
>>> +++ b/PVE/QemuServer/Makefile
>>> @@ -9,6 +9,7 @@ SOURCES=PCI.pm		\
>>>   	Monitor.pm	\
>>>   	Machine.pm	\
>>>   	CPUConfig.pm	\
>>> +	Drive.pm	\
>>>   
>>>   .PHONY: install
>>>   install: ${SOURCES}
>>> -- 
>>> 2.20.1
>>>
>>>
>>> _______________________________________________
>>> pve-devel mailing list
>>> pve-devel at pve.proxmox.com
>>> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>>
>>>
>> 
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel at pve.proxmox.com
>> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>> 
> 




More information about the pve-devel mailing list