[pve-devel] [PATCH docs 2/3] cloud init: use expected disk number in example
Starting from a storage with no disks for the VM ID in question, the imported disk will be called vm-9000-disk-0. Signed-off-by: Fiona Ebner --- qm-cloud-init.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qm-cloud-init.adoc b/qm-cloud-init.adoc index 25cd700..618d0f7 100644 --- a/qm-cloud-init.adoc +++ b/qm-cloud-init.adoc @@ -69,7 +69,7 @@ qm create 9000 --memory 2048 --net0 virtio,bridge=vmbr0 qm importdisk 9000 bionic-server-cloudimg-amd64.img local-lvm # finally attach the new disk to the VM as scsi drive -qm set 9000 --scsihw virtio-scsi-pci --scsi0 local-lvm:vm-9000-disk-1 +qm set 9000 --scsihw virtio-scsi-pci --scsi0 local-lvm:vm-9000-disk-0 NOTE: Ubuntu Cloud-Init images require the `virtio-scsi-pci` -- 2.30.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs 1/3] cloud init: use 'order' variant for 'boot' property
instead of the legacy 'boot: c'+bootdisk combination. Signed-off-by: Fiona Ebner --- qm-cloud-init.adoc | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/qm-cloud-init.adoc b/qm-cloud-init.adoc index 8363f10..25cd700 100644 --- a/qm-cloud-init.adoc +++ b/qm-cloud-init.adoc @@ -86,13 +86,12 @@ the Cloud-Init data to the VM. qm set 9000 --ide2 local-lvm:cloudinit -To be able to boot directly from the Cloud-Init image, set the -`bootdisk` parameter to `scsi0`, and restrict BIOS to boot from disk -only. This will speed up booting, because VM BIOS skips the testing for -a bootable CD-ROM. +To be able to boot directly from the Cloud-Init image, set the `boot` parameter +to `order=scsi0` to restrict BIOS to boot from this disk only. This will speed +up booting, because VM BIOS skips the testing for a bootable CD-ROM. -qm set 9000 --boot c --bootdisk scsi0 +qm set 9000 --boot order=scsi0 For many Cloud-Init images, it is required to configure a serial console and use -- 2.30.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH/RFC docs 3/3] cloud init: use 'import-from' disk syntax to avoid a step
Put the 'scsihw' part into the previous command, so that the rather complicated import command can stand on its own. Signed-off-by: Fiona Ebner --- Not entirely sure about this one, because it does make the commands a bit more involved. qm-cloud-init.adoc | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/qm-cloud-init.adoc b/qm-cloud-init.adoc index 618d0f7..b275d7a 100644 --- a/qm-cloud-init.adoc +++ b/qm-cloud-init.adoc @@ -62,14 +62,11 @@ image provided by Ubuntu at https://cloud-images.ubuntu.com. # download the image wget https://cloud-images.ubuntu.com/bionic/current/bionic-server-cloudimg-amd64.img -# create a new VM -qm create 9000 --memory 2048 --net0 virtio,bridge=vmbr0 +# create a new VM with VirtIO SCSI controller +qm create 9000 --memory 2048 --net0 virtio,bridge=vmbr0 --scsihw virtio-scsi-pci -# import the downloaded disk to local-lvm storage -qm importdisk 9000 bionic-server-cloudimg-amd64.img local-lvm - -# finally attach the new disk to the VM as scsi drive -qm set 9000 --scsihw virtio-scsi-pci --scsi0 local-lvm:vm-9000-disk-0 +# import the downloaded disk to the local-lvm storage, attaching it as a SCSI drive +qm set 9000 --scsi0 local-lvm:0,import-from=/path/to/bionic-server-cloudimg-amd64.img NOTE: Ubuntu Cloud-Init images require the `virtio-scsi-pci` -- 2.30.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH V5 pve-manager 1/2] fix #2822: add iscsi, lvm, lvmthin & zfs storage for all cluster nodes
thanks for the resend, i can now apply it ;) i played a bit around with this and i'm not completely happy with the ux here. having 'localhost' as text in the box is not really helpful because the user might not actually know what 'localhost' actuall is. also it's possible to empty the field, of which the meaning is also not really clear? my suggestion would be to set allowBlank to false, and by default select the current nodename (available in Proxmox.NodeName) That way the current node is preselected, and there is no misunderstanding which node is scanned at any time. i'd leave the restriction logic like it is though, because if the storage is available everywhere, you don't have to change the nodename at all but if you change it you want it probably restricted (like already mentioned in a previous review) aside from that the patches LGTM On 9/23/22 14:46, Stefan Hrdlicka wrote: This adds a dropdown box for iSCSI, LVM, LVMThin & ZFS storage options where a cluster node needs to be chosen. As default the current node is selected. It restricts the the storage to be only availabe on the selected node. Signed-off-by: Stefan Hrdlicka --- www/manager6/Makefile| 2 + www/manager6/form/ComboBoxSetStoreNode.js| 16 ++ www/manager6/form/StorageScanNodeSelector.js | 30 +++ www/manager6/storage/Base.js | 1 + www/manager6/storage/IScsiEdit.js| 32 +--- www/manager6/storage/LVMEdit.js | 29 +-- www/manager6/storage/LvmThinEdit.js | 52 +++- www/manager6/storage/ZFSPoolEdit.js | 28 +-- 8 files changed, 166 insertions(+), 24 deletions(-) create mode 100644 www/manager6/form/ComboBoxSetStoreNode.js create mode 100644 www/manager6/form/StorageScanNodeSelector.js diff --git a/www/manager6/Makefile b/www/manager6/Makefile index d16770b1..81f5e5d8 100644 --- a/www/manager6/Makefile +++ b/www/manager6/Makefile @@ -27,6 +27,7 @@ JSSRC= \ form/CalendarEvent.js \ form/CephPoolSelector.js\ form/CephFSSelector.js \ + form/ComboBoxSetStoreNode.js\ form/CompressionSelector.js \ form/ContentTypeSelector.js \ form/ControllerSelector.js \ @@ -62,6 +63,7 @@ JSSRC= \ form/SecurityGroupSelector.js \ form/SnapshotSelector.js\ form/SpiceEnhancementSelector.js\ + form/StorageScanNodeSelector.js \ form/StorageSelector.js \ form/TFASelector.js \ form/TokenSelector.js \ diff --git a/www/manager6/form/ComboBoxSetStoreNode.js b/www/manager6/form/ComboBoxSetStoreNode.js new file mode 100644 index ..3490ddd7 --- /dev/null +++ b/www/manager6/form/ComboBoxSetStoreNode.js @@ -0,0 +1,16 @@ +Ext.define('PVE.form.ComboBoxSetStoreNode', { +extend: 'Ext.form.field.ComboBox', +config: { + apiBaseUrl: '/api2/json/nodes/', + apiSuffix: '', +}, + +setNodeName: function(value) { + let me = this; + value ||= Proxmox.NodeName; + + me.getStore().getProxy().setUrl(`${me.apiBaseUrl}${value}${me.apiSuffix}`); + this.clearValue(); +}, + +}); diff --git a/www/manager6/form/StorageScanNodeSelector.js b/www/manager6/form/StorageScanNodeSelector.js new file mode 100644 index ..80188707 --- /dev/null +++ b/www/manager6/form/StorageScanNodeSelector.js @@ -0,0 +1,30 @@ +Ext.define('PVE.form.StorageScanNodeSelector', { +extend: 'PVE.form.NodeSelector', +xtype: 'pveStorageScanNodeSelector', + +name: 'storageScanNode', +itemId: 'pveStorageScanNodeSelector', +fieldLabel: gettext('Scan node'), +allowBlank: true, +disallowedNodes: undefined, +autoSelect: false, +submitValue: false, +value: 'localhost', +autoEl: { + tag: 'div', + 'data-qtip': gettext('Scan for available storages on the selected node'), +}, +triggers: { + clear: { + handler: function() { + let me = this; + me.setValue('localhost'); + }, + }, +}, +setValue: function(value) { + let me = this; + me.callParent([value]); + me.triggers.clear.setVisible(me.triggers.clear.isVisible() && value !== 'localhost'); +}, +}); diff --git a/www/manager6/storage/Base.js b/www/manager6/storage/Base.js index 7f6d7a09..1df7a8dd 100644 --- a/www/manager6/storage/Base.js +++ b/www/manager6/storage/Base.js @@ -36,6 +36,7 @@ Ext.define('PVE.panel.StorageBase', { { xtype: 'pveNodeSelector',
[pve-devel] applied: [PATCH http-server v2 2/7] acknowledge content-disposition header
Am 07/09/2022 um 10:56 schrieb Daniel Tschlatscher: > Acknowledging the Content-Disposition header makes it possible for the > backend to tell the browser whether a file should be downloaded, > rather than displayed inline, and what it's default name should be. > > Signed-off-by: Daniel Tschlatscher > --- > src/PVE/APIServer/AnyEvent.pm | 3 +++ > 1 file changed, 3 insertions(+) > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH v6 storage 1/1] (remote) export: check and untaint format
Am 28/09/2022 um 14:50 schrieb Fabian Grünbichler: > this format comes from the remote cluster, so it might not be supported > on the source side - checking whether it's known (as additional > safeguard) and untainting (to avoid open3 failure) is required. > > Signed-off-by: Fabian Grünbichler > --- > > Notes: > v6: new > > PVE/CLI/pvesm.pm | 6 ++ > PVE/Storage.pm | 9 + > 2 files changed, 11 insertions(+), 4 deletions(-) > > applied, with a small code style fixup (`($foo->@*)[0]` vs `$foo->[0]`) squashed in, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server 2/2] reuse existing cloud-init disks
When a disk exists but is not referenced in the config, it will be reused instead of dying during the attempt to allocate the disk. Signed-off-by: Mira Limbeck --- This patch is not required to fix the rollback code, but might be nice to have in addition since there will still be some users with cloud-init disks left on the storage. PVE/API2/Qemu.pm | 23 ++- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm index 3ec31c2..73d6ab9 100644 --- a/PVE/API2/Qemu.pm +++ b/PVE/API2/Qemu.pm @@ -348,15 +348,28 @@ my $create_disks = sub { $fmt = $disk->{format} // "raw"; } - # Initial disk created with 4 MB and aligned to 4MB on regeneration - my $ci_size = PVE::QemuServer::Cloudinit::CLOUDINIT_DISK_SIZE; - my $volid = PVE::Storage::vdisk_alloc($storecfg, $storeid, $vmid, $fmt, $name, $ci_size/1024); - $disk->{file} = $volid; + # since there was an issue with the rollback code not deleting cloud-init disks + # there can be a case where leftover cloud-init disks are still on the storage. + # since those will be overwritten anyway on each boot, we can just reuse them + # if they already exist + my $reused; + my $ci_disk_found = PVE::Storage::vdisk_list($storecfg, $storeid, $vmid, ["$storeid:$name"], 'images'); + if ($ci_disk_found->{$storeid} && scalar($ci_disk_found->{$storeid}->@*)) { + print "Cloud-Init disk $name already exists on the storage '$storeid', reusing it.\n"; + my $ci_disk = $ci_disk_found->{$storeid}[0]; + $disk->{file} = $ci_disk->{volid}; + $reused = 1; + } else { + # Initial disk created with 4 MB and aligned to 4MB on regeneration + my $ci_size = PVE::QemuServer::Cloudinit::CLOUDINIT_DISK_SIZE; + my $volid = PVE::Storage::vdisk_alloc($storecfg, $storeid, $vmid, $fmt, $name, $ci_size/1024); + $disk->{file} = $volid; + } $disk->{media} = 'cdrom'; push @$vollist, $volid; delete $disk->{format}; # no longer needed $res->{$ds} = PVE::QemuServer::print_drive($disk); - print "$ds: successfully created disk '$res->{$ds}'\n"; + print "$ds: successfully created disk '$res->{$ds}'\n" if !$reused; } elsif ($volid =~ $NEW_DISK_RE) { my ($storeid, $size) = ($2 || $default_storage, $3); die "no storage ID specified (and no default storage)\n" if !$storeid; -- 2.30.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server 1/2] fix #4201: delete cloud-init disk on rollback
If the config doesn't contain the cloud-init disk anymore after the rollback, we have to clean it up since otherwise no further disk can be attached unless the one still existing on the storage is deleted. Signed-off-by: Mira Limbeck --- PVE/QemuConfig.pm | 34 ++ 1 file changed, 34 insertions(+) diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm index 482c7ab..4a744cc 100644 --- a/PVE/QemuConfig.pm +++ b/PVE/QemuConfig.pm @@ -419,6 +419,17 @@ sub __snapshot_rollback_hook { if ($prepare) { # we save the machine of the current config $data->{oldmachine} = $conf->{machine}; + + # keep info about cloudinit disk in the config before the rollback + # will be used to later keep or delete possible leftover cloudinit disks + # since cloudinit disks are not part of the snapshots + $class->foreach_volume($conf, sub { + my ($ds, $drive) = @_; + + return if !PVE::QemuServer::drive_is_cloudinit($drive); + + $data->{cloudinit} = $drive; + }); } else { # if we have a 'runningmachine' entry in the snapshot we use that # for the forcemachine parameter, else we use the old logic @@ -446,6 +457,29 @@ sub __snapshot_rollback_hook { # re-initializing its random number generator $conf->{vmgenid} = PVE::QemuServer::generate_uuid(); } + + # config before rollback contained a cloudinit disk + # check if that is still the case after the rollback + if ($data->{cloudinit}) { + my $found = 0; + $class->foreach_volume($conf, sub { + my ($ds, $drive) = @_; + + if (PVE::QemuServer::drive_is_cloudinit($drive)) { + $found = 1; + last; + } + }); + + # missing cloudinit disk after rollback + # clean up existing cloudinit disk + if (!$found) { +print "removing unreferenced cloud-init disk $data->{cloudinit}->{file}\n"; + + my $storecfg = PVE::Storage::config(); + PVE::Storage::vdisk_free($storecfg, $data->{cloudinit}->{file}); + } + } } return; -- 2.30.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH v2 http-server 1/2] AnyEvent: whitespace fix
Am 13/05/2022 um 15:48 schrieb Matthias Heiserer: > and remove unnecessary parentheses. > > Signed-off-by: Matthias Heiserer > --- > > Changes from v1: > new patch > > src/PVE/APIServer/AnyEvent.pm | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH v2 http-server 2/2] AnyEvent: Fix #3990 - make small files uploadable
Am 13/05/2022 um 15:49 schrieb Matthias Heiserer: > == The problem > Upload of files smaller than ~16kb failed. > This was because the code assumed that it would be called > several times, and each time would do a certain action. > When the whole file was in the buffer, this failed because > the function would try parssing the first part in the payload and > then return, instead of parsing the rest of the available data. > > == Why not just modifying the current code a bit? > The code had a lot of nested control statements and a > non-intuitive control flow (phase 0->2->1->1->1 and so on). > > The way the phases and buffer content were checked made it > rather difficult to just fix a few lines. > > == What was changed > * Part headers are parsed with a single regex line each, > which improves code readability. > > * Parsing the content is done in order, so even if the whole data is in the > buffer, > it can be read in one go. Files of arbitrary sizes can be uploaded. > > == Tested with > * Uploaded 0B, 1B, 14KB, 16KB, 1GB, 10GB, 20GB files > > * Tested with all checksums and without > > * Tested on firefox, chromium, and pvesh > > I didn't do any fuzzing or automated upload testing. > > == Drawbacks & Potential issues > * Part headers are hardcoded, adding new ones requries modifying this file > > == does not fix > * upload can still time out > > Signed-off-by: Matthias Heiserer > --- > > Note: > Regarding trim, I forgot to answer the mail. > Trim is imo a good name, as several languagues (e.g. rust, javascript) > use trim to mean mean "remove all whitespace, including newlines and such". > I can send a v3 if that's a problem. > > Changes from v1: > * fix whitespace in separate patch > * move trim into inline closure > * correctly call trim > * replace [^\S] with \S in regexes > * improve trim regex: don't replace string > * check for phase 1 once > * remove regex comment > > src/PVE/APIServer/AnyEvent.pm | 146 ++ > 1 file changed, 76 insertions(+), 70 deletions(-) > > applied, with slightly reworking the commit messages subject and Daniel's T-b tag, thanks to both! Note that I made some follow ups trying to improve a few things of your change and the existing code. E.g.: $string in trim wasn't used anywhere, same for $eof in phase 2. The content-disposition extraction could be factored in a small closure to reduce code size and (IMO) legibility, and some other smaller style/formatting stuff. None of that was a blocker, and due to the age of this series (sorry for that!) I really did not want to bother for a v3 and applied changes myself. I re-tested it with various sizes and functionality (checksum), but an additional pair of eyes would be appreciated to ensure no regression snuck in. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel