On 16.03.20 10:10, Fabian Grünbichler wrote:
On March 12, 2020 11:19 am, Fabian Ebner wrote:
and make it match with what parse_drive does. Even though the 'real' format
was pve-volume-id, callers already expected that parse_drive returns a hash
with a valid 'file' key (e.g. PVE/API2/Qemu.pm:1147ff).

Signed-off-by: Fabian Ebner <f.eb...@proxmox.com>
---

This is the last patch left over from the series
refactoring the drive code [0], although not directly related.

[0]: https://pve.proxmox.com/pipermail/pve-devel/2020-March/042007.html

Changes from v3:
     * make sure the format describes a hash with a 'file' key
       as that's what callers expect

  PVE/QemuServer/Drive.pm | 15 +++++++++++++--
  1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
index e927abf..d412147 100644
--- a/PVE/QemuServer/Drive.pm
+++ b/PVE/QemuServer/Drive.pm
@@ -355,9 +355,20 @@ for (my $i = 0; $i < $MAX_VIRTIO_DISKS; $i++)  {
$drivedesc_hash->{efidisk0} = $efidisk_desc; +my $unused_fmt = {
+    volume => { alias => 'file' },
+    file => {
+       type => 'string',
+       format => 'pve-volume-id',
+       default_key => 1,
+       format_description => 'volume',
+       description => "The drive's backing volume.",
+    },
+};
+
  our $unuseddesc = {
      optional => 1,
-    type => 'string', format => 'pve-volume-id',
+    type => 'string', format => $unused_fmt,
      description => "Reference to unused volumes. This is used internally, and 
should not be modified manually.",
  };
@@ -418,7 +429,7 @@ sub parse_drive {
        return undef;
      }
- my $desc = $key =~ /^unused\d+$/ ? $alldrive_fmt
+    my $desc = $key =~ /^unused\d+$/ ? $unuseddesc->{format}
                                       : $drivedesc_hash->{$key}->{format};

couldn't we just put the unused schema into $drivedesc_hash as well?

is_valid_drivename would need to skip them[1], but we'd have all the disk
schema in one place.


Ok, I'll check the call sites and do a follow-up for this.

that being said - the patch as is LGTM and the above can be done as
follow-up just as well:

Reviewed-By: Fabian Grünbichler <f.gruenbich...@proxmox.com>

1: in general? or just by default? maybe there are call sites that would
benefit/get simpler by including unused as well..
 >
      if (!$desc) {
        warn "invalid drive key: $key\n";
--
2.20.1


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



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


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

Reply via email to