On Sat, Jun 06, 2020 at 07:56:20PM +0200, Thomas Lamprecht wrote:
> On 5/22/20 12:08 PM, Dominic Jäger wrote:
> > I'd appreciate a few hints for the importdisk GUI feature.
> >
> > Yes or no to automatically adding the drive as active (not unused)?
>
> Well, IMO makes sense but I'd add an parameter for either a config ID (scsi0,
> sata2,
> ...) which it is added if free, and/or the bus (controller) where we then
> select the
> next free slot if any.
Thank you! Makes sense. I'll add that and, of course, also the rest of the
feedback.
> > + The image format has to be supported by qemu-img(1).",
>
> please use concatenated multi line strings for description, i.e.:
>
> description => "Import an external disk image into a VM. "
> ."The image format has to be supported by qemu-img(1).",
>
> > + permissions => {
> > + description => "You need 'VM.Config.Disk' permissions on /vms/{vmid},
> > + and 'Datastore.AllocateSpace' permissions on the storage.",
> > + check => [ 'and',
> > + ['perm', '/vms/{vmid}', [ 'VM.Config.Disk' ]],
> > + ['perm', '/storage/{storage}', [ 'Datastore.AllocateSpace'
> > ]],
> > + ],
> > + },
> > + parameters => {
> > + additionalProperties => 0,
> > + properties => PVE::QemuServer::json_config_properties ({
>
> hmm, doesn't this puts _all_ of our VM config properties in here?? Most
> probably
> do not make sense, only some disk/bus related ones, I'd say.
I'll have to take a closer look at that.
> > + foreach my $opt (keys %$param) {
> > + die ("Die because multiple drives\n") if defined($driveX);
>
> nit: no parenthesis in die
>
> > +
> > + my $message = $drive ? "to drive $drive->{'file'} on" : 'as unused
> > drive to';
> > + print "Importing disk '$source' " . $message . " VM $vmid ...\n";
>
> FYI: double quoted strings allow $variable interpolation, so you do not need
> to
> concat this extra.
Don't know why I missed this...
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
