public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [WIP qemu-server, manager 0/3] nvme-emulation, current state
@ 2024-07-31 13:16 Aaron Lauterer
  2024-07-31 13:40 ` [pve-devel] [WIP qemu-server 1/3] fix #2255: add support for nvme emulation Aaron Lauterer
  0 siblings, 1 reply; 5+ messages in thread
From: Aaron Lauterer @ 2024-07-31 13:16 UTC (permalink / raw)
  To: pve-devel

I picked up the older patch series to add NVME as another disk type [0].
After  rebasing the old patches through ~4 years of changes and adding a
few follow-up changes I was able to get hot-plugging working. Though
further work is needed to make it stable.

The big blocker is still live-migration. More details can be read on my
own patch on top <nvme: make it somewhat work with current version>.

I am sending this series here to document the current efforts and to
make it easier for others as well as these patches should apply to the
current code base.

This is not meant to be applied to reviewed regarding the code quality
;)


[0] https://lists.proxmox.com/pipermail/pve-devel/2020-May/043540.html

qemu-server: Aaron Lauterer (1):
  nvme: make it somewhat work with current version

Oguz Bektas (1):
  fix #2255: add support for nvme emulation

 PVE/QemuServer.pm       | 26 +++++++++++++++++++++++---
 PVE/QemuServer/Drive.pm | 22 ++++++++++++++++++++++
 2 files changed, 45 insertions(+), 3 deletions(-)


manager: Oguz Bektas (1):
  gui: add nvme as a bus type for creating disks

 www/manager6/Utils.js                   | 3 ++-
 www/manager6/form/BusTypeSelector.js    | 2 ++
 www/manager6/form/ControllerSelector.js | 2 +-
 www/manager6/qemu/CloudInit.js          | 4 ++--
 www/mobile/QemuSummary.js               | 2 +-
 5 files changed, 8 insertions(+), 5 deletions(-)

-- 
2.39.2



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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [pve-devel] [WIP qemu-server 1/3] fix #2255: add support for nvme emulation
  2024-07-31 13:16 [pve-devel] [WIP qemu-server, manager 0/3] nvme-emulation, current state Aaron Lauterer
@ 2024-07-31 13:40 ` Aaron Lauterer
  2024-07-31 13:40   ` [pve-devel] [WIP qemu-server 2/3] nvme: make it somewhat work with current version Aaron Lauterer
                     ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Aaron Lauterer @ 2024-07-31 13:40 UTC (permalink / raw)
  To: pve-devel

From: Oguz Bektas <o.bektas@proxmox.com>

now we can add nvme drives;

nvme0: local-lvm:vm-103-disk-0,size=32G

or

qm set VMID --nvme0 local-lvm:32

max number is 8 for now, as most real hardware has 1-3 nvme slots and
can have a few more with pcie. most cases won't go over 8, if there's an
actual usecase at some point this can always be increased without
breaking anything (although the same isn't valid for decreasing it).

Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
---
 PVE/QemuServer.pm       | 23 ++++++++++++++++++++---
 PVE/QemuServer/Drive.pm | 21 +++++++++++++++++++++
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index b26da50..7d8d75b 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -437,7 +437,7 @@ EODESC
 	optional => 1,
 	type => 'string', format => 'pve-qm-bootdisk',
 	description => "Enable booting from specified disk. Deprecated: Use 'boot: order=foo;bar' instead.",
-	pattern => '(ide|sata|scsi|virtio)\d+',
+	pattern => '(ide|sata|scsi|virtio|nvme)\d+',
     },
     smp => {
 	optional => 1,
@@ -1433,7 +1433,10 @@ sub print_drivedevice_full {
 		$device .= ",product=$product";
 	    }
 	}
-
+    } elsif ($drive->{interface} eq 'nvme') {
+	my $path = $drive->{file};
+	$drive->{serial} //= "$drive->{interface}$drive->{index}"; # serial is mandatory for nvme
+	$device = "nvme,drive=drive-$drive->{interface}$drive->{index},id=$drive->{interface}$drive->{index}";
     } elsif ($drive->{interface} eq 'ide' || $drive->{interface} eq 'sata') {
 	my $maxdev = ($drive->{interface} eq 'sata') ? $PVE::QemuServer::Drive::MAX_SATA_DISKS : 2;
 	my $controller = int($drive->{index} / $maxdev);
@@ -2396,7 +2399,7 @@ sub parse_vm_config {
 	    } else {
 		$key = 'ide2' if $key eq 'cdrom';
 		my $fmt = $confdesc->{$key}->{format};
-		if ($fmt && $fmt =~ /^pve-qm-(?:ide|scsi|virtio|sata)$/) {
+		if ($fmt && $fmt =~ /^pve-qm-(?:ide|scsi|virtio|sata|nvme)$/) {
 		    my $v = parse_drive($key, $value);
 		    if (my $volid = filename_to_volume_id($vmid, $v->{file}, $v->{media})) {
 			$v->{file} = $volid;
@@ -4289,6 +4292,17 @@ sub vm_deviceplug {
 	    warn $@ if $@;
 	    die $err;
         }
+    } elsif ($deviceid =~ m/^(nvme)(\d+)$/) {
+
+	qemu_driveadd($storecfg, $vmid, $device);
+
+	my $devicefull = print_drivedevice_full($storecfg, $conf, $vmid, $device, $arch, $machine_type);
+	eval { qemu_deviceadd($vmid, $devicefull); };
+	if (my $err = $@) {
+	    eval { qemu_drivedel($vmid, $deviceid); };
+	    warn $@ if $@;
+	    die $err;
+        }
     } elsif ($deviceid =~ m/^(net)(\d+)$/) {
 	return if !qemu_netdevadd($vmid, $conf, $arch, $device, $deviceid);
 
@@ -4361,6 +4375,9 @@ sub vm_deviceunplug {
 
 	qemu_iothread_del($vmid, "virtioscsi$device->{index}", $device)
 	    if $conf->{scsihw} && ($conf->{scsihw} eq 'virtio-scsi-single');
+    } elsif ($deviceid =~ m/^(nvme)(\d+)$/) {
+	qemu_devicedel($vmid, $deviceid);
+	qemu_drivedel($vmid, $deviceid);
     } elsif ($deviceid =~ m/^(net)(\d+)$/) {
 	qemu_devicedel($vmid, $deviceid);
 	qemu_devicedelverify($vmid, $deviceid);
diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
index 6e98c09..f05ad26 100644
--- a/PVE/QemuServer/Drive.pm
+++ b/PVE/QemuServer/Drive.pm
@@ -33,6 +33,7 @@ PVE::JSONSchema::register_standard_option('pve-qm-image-format', {
 
 my $MAX_IDE_DISKS = 4;
 my $MAX_SCSI_DISKS = 31;
+my $MAX_NVME_DISKS = 8;
 my $MAX_VIRTIO_DISKS = 16;
 our $MAX_SATA_DISKS = 6;
 our $MAX_UNUSED_DISKS = 256;
@@ -315,6 +316,20 @@ my $scsidesc = {
 };
 PVE::JSONSchema::register_standard_option("pve-qm-scsi", $scsidesc);
 
+my $nvme_fmt = {
+    %drivedesc_base,
+    %ssd_fmt,
+    %wwn_fmt,
+};
+
+my $nvmedesc = {
+    optional => 1,
+    type => 'string', format => $nvme_fmt,
+    description => "Use volume as NVME disk (n is 0 to " . ($MAX_NVME_DISKS -1) . ").",
+};
+
+PVE::JSONSchema::register_standard_option("pve-qm-nvme", $nvmedesc);
+
 my $sata_fmt = {
     %drivedesc_base,
     %ssd_fmt,
@@ -515,6 +530,11 @@ for (my $i = 0; $i < $MAX_SCSI_DISKS; $i++)  {
     $drivedesc_hash_with_alloc->{"scsi$i"} = $desc_with_alloc->('scsi', $scsidesc);
 }
 
+for (my $i = 0; $i < $MAX_NVME_DISKS; $i++)  {
+    $drivedesc_hash->{"nvme$i"} = $nvmedesc;
+}
+
+
 for (my $i = 0; $i < $MAX_VIRTIO_DISKS; $i++)  {
     $drivedesc_hash->{"virtio$i"} = $virtiodesc;
     $drivedesc_hash_with_alloc->{"virtio$i"} = $desc_with_alloc->('virtio', $virtiodesc);
@@ -541,6 +561,7 @@ sub valid_drive_names {
             (map { "scsi$_" } (0 .. ($MAX_SCSI_DISKS - 1))),
             (map { "virtio$_" } (0 .. ($MAX_VIRTIO_DISKS - 1))),
             (map { "sata$_" } (0 .. ($MAX_SATA_DISKS - 1))),
+            (map { "nvme$_" } (0 .. ($MAX_NVME_DISKS - 1))),
             'efidisk0',
             'tpmstate0');
 }
-- 
2.39.2



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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [pve-devel] [WIP qemu-server 2/3] nvme: make it somewhat work with current version
  2024-07-31 13:40 ` [pve-devel] [WIP qemu-server 1/3] fix #2255: add support for nvme emulation Aaron Lauterer
@ 2024-07-31 13:40   ` Aaron Lauterer
  2024-07-31 13:40   ` [pve-devel] [WIP manager 3/3] gui: add nvme as a bus type for creating disks Aaron Lauterer
  2024-07-31 15:30   ` [pve-devel] [WIP qemu-server 1/3] fix #2255: add support for nvme emulation Thomas Lamprecht
  2 siblings, 0 replies; 5+ messages in thread
From: Aaron Lauterer @ 2024-07-31 13:40 UTC (permalink / raw)
  To: pve-devel

this patch is not meant to be applied but captures my efforts to make
the old series work (somewhat).

This way I was able to achieve hot-plugging on an i440 VM.

Hot plugging on a q35 VM did not work in my tests, but I think it is
related to how the NVME device was attached to the PCI tree of the VM.
By controlling this more directly, we should be able to attach it to a
PCI port/bridge that allows hot-plugging.

For some reason that I need to investigate further, giving the nvme
device the id `nvmeX` will cause issues when unplugging. It seems our
code will still see the `drive-nvmeX` drive and will add it to the list
of devices present. Therefore, the unplug code will think it did not
remove successfully. I worked around that for now by calling the nvme
device `id=nvmecontX` instead of `id=nvmeX`.

Live migration is still a blocker as the NVME QEMU device does not allow
it.
It fails with the following errors:

2024-07-31 09:31:14 migrate uri => unix:/run/qemu-server/102.migrate failed: VM 102 qmp command 'migrate' failed - State blocked by non-migratable device '0000:00:04.0/nvme'
2024-07-31 09:31:15 ERROR: online migrate failure - VM 102 qmp command 'migrate' failed - State blocked by non-migratable device '0000:00:04.0/nvme'

We need to check (with upstream) what is missing. Also if there are
plans upstream to implement the missing pieces, or if we could do it
ourselves.

I found the following older thread on the qemu mailing list:
https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg05788.html

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 PVE/QemuServer.pm       | 15 +++++++++------
 PVE/QemuServer/Drive.pm |  1 +
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 7d8d75b..6dc3eb2 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -1436,7 +1436,7 @@ sub print_drivedevice_full {
     } elsif ($drive->{interface} eq 'nvme') {
 	my $path = $drive->{file};
 	$drive->{serial} //= "$drive->{interface}$drive->{index}"; # serial is mandatory for nvme
-	$device = "nvme,drive=drive-$drive->{interface}$drive->{index},id=$drive->{interface}$drive->{index}";
+	$device = "nvme,drive=drive-$drive->{interface}$drive->{index},id=$drive->{interface}cont$drive->{index}";
     } elsif ($drive->{interface} eq 'ide' || $drive->{interface} eq 'sata') {
 	my $maxdev = ($drive->{interface} eq 'sata') ? $PVE::QemuServer::Drive::MAX_SATA_DISKS : 2;
 	my $controller = int($drive->{index} / $maxdev);
@@ -4292,11 +4292,10 @@ sub vm_deviceplug {
 	    warn $@ if $@;
 	    die $err;
         }
-    } elsif ($deviceid =~ m/^(nvme)(\d+)$/) {
-
+    } elsif ($deviceid =~ m/^nvme(\d+)$/) {
 	qemu_driveadd($storecfg, $vmid, $device);
 
-	my $devicefull = print_drivedevice_full($storecfg, $conf, $vmid, $device, $arch, $machine_type);
+	my $devicefull = print_drivedevice_full($storecfg, $conf, $vmid, $device, undef, $arch, $machine_type);
 	eval { qemu_deviceadd($vmid, $devicefull); };
 	if (my $err = $@) {
 	    eval { qemu_drivedel($vmid, $deviceid); };
@@ -4375,8 +4374,12 @@ sub vm_deviceunplug {
 
 	qemu_iothread_del($vmid, "virtioscsi$device->{index}", $device)
 	    if $conf->{scsihw} && ($conf->{scsihw} eq 'virtio-scsi-single');
-    } elsif ($deviceid =~ m/^(nvme)(\d+)$/) {
-	qemu_devicedel($vmid, $deviceid);
+    } elsif ($deviceid =~ m/^nvme(\d+)$/) {
+	my $device = parse_drive($deviceid, $conf->{$deviceid});
+
+	my $nvmecont = "nvmecont$1";
+	qemu_devicedel($vmid, $nvmecont);
+	qemu_devicedelverify($vmid, $nvmecont);
 	qemu_drivedel($vmid, $deviceid);
     } elsif ($deviceid =~ m/^(net)(\d+)$/) {
 	qemu_devicedel($vmid, $deviceid);
diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
index f05ad26..b055674 100644
--- a/PVE/QemuServer/Drive.pm
+++ b/PVE/QemuServer/Drive.pm
@@ -532,6 +532,7 @@ for (my $i = 0; $i < $MAX_SCSI_DISKS; $i++)  {
 
 for (my $i = 0; $i < $MAX_NVME_DISKS; $i++)  {
     $drivedesc_hash->{"nvme$i"} = $nvmedesc;
+    $drivedesc_hash_with_alloc->{"nvme$i"} = $desc_with_alloc->('nvme', $nvmedesc);
 }
 
 
-- 
2.39.2



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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [pve-devel] [WIP manager 3/3] gui: add nvme as a bus type for creating disks
  2024-07-31 13:40 ` [pve-devel] [WIP qemu-server 1/3] fix #2255: add support for nvme emulation Aaron Lauterer
  2024-07-31 13:40   ` [pve-devel] [WIP qemu-server 2/3] nvme: make it somewhat work with current version Aaron Lauterer
@ 2024-07-31 13:40   ` Aaron Lauterer
  2024-07-31 15:30   ` [pve-devel] [WIP qemu-server 1/3] fix #2255: add support for nvme emulation Thomas Lamprecht
  2 siblings, 0 replies; 5+ messages in thread
From: Aaron Lauterer @ 2024-07-31 13:40 UTC (permalink / raw)
  To: pve-devel

From: Oguz Bektas <o.bektas@proxmox.com>

Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
---
 www/manager6/Utils.js                   | 3 ++-
 www/manager6/form/BusTypeSelector.js    | 2 ++
 www/manager6/form/ControllerSelector.js | 2 +-
 www/manager6/qemu/CloudInit.js          | 4 ++--
 www/mobile/QemuSummary.js               | 2 +-
 5 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index db86fa9a..aba72135 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -13,7 +13,7 @@ Ext.define('PVE.Utils', {
 
     toolkit: undefined, // (extjs|touch), set inside Toolkit.js
 
-    bus_match: /^(ide|sata|virtio|scsi)(\d+)$/,
+    bus_match: /^(ide|sata|virtio|scsi|nvme)\d+$/,
 
     log_severity_hash: {
 	0: "panic",
@@ -1560,6 +1560,7 @@ Ext.define('PVE.Utils', {
 	ide: 4,
 	sata: 6,
 	scsi: 31,
+	nvme: 8,
 	virtio: 16,
 	unused: 256,
     },
diff --git a/www/manager6/form/BusTypeSelector.js b/www/manager6/form/BusTypeSelector.js
index 0f040229..fbc6f222 100644
--- a/www/manager6/form/BusTypeSelector.js
+++ b/www/manager6/form/BusTypeSelector.js
@@ -16,6 +16,8 @@ Ext.define('PVE.form.BusTypeSelector', {
 
 	me.comboItems.push(['scsi', 'SCSI']);
 
+	me.comboItems.push(['nvme', 'NVMe']);
+
 	if (me.withUnused) {
 	    me.comboItems.push(['unused', 'Unused']);
 	}
diff --git a/www/manager6/form/ControllerSelector.js b/www/manager6/form/ControllerSelector.js
index d7c2625d..8c766598 100644
--- a/www/manager6/form/ControllerSelector.js
+++ b/www/manager6/form/ControllerSelector.js
@@ -39,7 +39,7 @@ Ext.define('PVE.form.ControllerSelector', {
 		deviceid.setValue(2);
 		return;
 	    }
-	    clist = ['ide', 'scsi', 'sata'];
+	    clist = ['ide', 'scsi', 'sata', 'nvme'];
 	} else {
 	    // in most cases we want to add a disk to the same controller we previously used
 	    clist = PVE.Utils.sortByPreviousUsage(me.vmconfig);
diff --git a/www/manager6/qemu/CloudInit.js b/www/manager6/qemu/CloudInit.js
index 49519726..4ea58641 100644
--- a/www/manager6/qemu/CloudInit.js
+++ b/www/manager6/qemu/CloudInit.js
@@ -135,7 +135,7 @@ Ext.define('PVE.qemu.CloudInit', {
 	    var id = record.data.key;
 	    var value = record.data.value;
 	    var ciregex = new RegExp("vm-" + me.pveSelNode.data.vmid + "-cloudinit");
-		if (id.match(/^(ide|scsi|sata)\d+$/) && ciregex.test(value)) {
+		if (id.match(/^(ide|scsi|sata|nvme)\d+$/) && ciregex.test(value)) {
 		    found = id;
 		    me.ciDriveId = found;
 		    me.ciDrive = value;
@@ -330,7 +330,7 @@ Ext.define('PVE.qemu.CloudInit', {
 	    };
 	}
 
-	PVE.Utils.forEachBus(['ide', 'scsi', 'sata'], function(type, id) {
+	PVE.Utils.forEachBus(['ide', 'scsi', 'sata', 'nvme'], function(type, id) {
 	    me.rows[type+id] = {
 		visible: false,
 	    };
diff --git a/www/mobile/QemuSummary.js b/www/mobile/QemuSummary.js
index c416ea7d..a484fcb6 100644
--- a/www/mobile/QemuSummary.js
+++ b/www/mobile/QemuSummary.js
@@ -12,7 +12,7 @@ Ext.define('PVE.QemuSummary', {
 
     config_keys: [
 	'name', 'memory', 'sockets', 'cores', 'ostype', 'bootdisk', /^net\d+/,
-	/^ide\d+/, /^virtio\d+/, /^sata\d+/, /^scsi\d+/, /^unused\d+/,
+	/^ide\d+/, /^virtio\d+/, /^sata\d+/, /^scsi\d+/, /^nvme\d+/, /^unused\d+/
     ],
 
     initialize: function() {
-- 
2.39.2



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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [pve-devel] [WIP qemu-server 1/3] fix #2255: add support for nvme emulation
  2024-07-31 13:40 ` [pve-devel] [WIP qemu-server 1/3] fix #2255: add support for nvme emulation Aaron Lauterer
  2024-07-31 13:40   ` [pve-devel] [WIP qemu-server 2/3] nvme: make it somewhat work with current version Aaron Lauterer
  2024-07-31 13:40   ` [pve-devel] [WIP manager 3/3] gui: add nvme as a bus type for creating disks Aaron Lauterer
@ 2024-07-31 15:30   ` Thomas Lamprecht
  2 siblings, 0 replies; 5+ messages in thread
From: Thomas Lamprecht @ 2024-07-31 15:30 UTC (permalink / raw)
  To: Proxmox VE development discussion, Aaron Lauterer

Am 31/07/2024 um 15:40 schrieb Aaron Lauterer:
> From: Oguz Bektas <o.bektas@proxmox.com>
> 
> now we can add nvme drives;
> 
> nvme0: local-lvm:vm-103-disk-0,size=32G
> 
> or
> 
> qm set VMID --nvme0 local-lvm:32
> 
> max number is 8 for now, as most real hardware has 1-3 nvme slots and
> can have a few more with pcie. most cases won't go over 8, if there's an
> actual usecase at some point this can always be increased without
> breaking anything (although the same isn't valid for decreasing it).
> 
> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>

A S-o-b from you hear would be good to have, you can also describe the
changes done on top of original patch like:

Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
 [ AL: rebased, ... ]
Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>


> ---
>  PVE/QemuServer.pm       | 23 ++++++++++++++++++++---
>  PVE/QemuServer/Drive.pm | 21 +++++++++++++++++++++
>  2 files changed, 41 insertions(+), 3 deletions(-)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index b26da50..7d8d75b 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -437,7 +437,7 @@ EODESC
>  	optional => 1,
>  	type => 'string', format => 'pve-qm-bootdisk',
>  	description => "Enable booting from specified disk. Deprecated: Use 'boot: order=foo;bar' instead.",
> -	pattern => '(ide|sata|scsi|virtio)\d+',
> +	pattern => '(ide|sata|scsi|virtio|nvme)\d+',
>      },
>      smp => {
>  	optional => 1,
> @@ -1433,7 +1433,10 @@ sub print_drivedevice_full {
>  		$device .= ",product=$product";
>  	    }
>  	}
> -
> +    } elsif ($drive->{interface} eq 'nvme') {
> +	my $path = $drive->{file};

^- this variable seems to be unused? 

> +	$drive->{serial} //= "$drive->{interface}$drive->{index}"; # serial is mandatory for nvme
> +	$device = "nvme,drive=drive-$drive->{interface}$drive->{index},id=$drive->{interface}$drive->{index}";
>      } elsif ($drive->{interface} eq 'ide' || $drive->{interface} eq 'sata') {
>  	my $maxdev = ($drive->{interface} eq 'sata') ? $PVE::QemuServer::Drive::MAX_SATA_DISKS : 2;
>  	my $controller = int($drive->{index} / $maxdev);
> @@ -2396,7 +2399,7 @@ sub parse_vm_config {
>  	    } else {
>  		$key = 'ide2' if $key eq 'cdrom';
>  		my $fmt = $confdesc->{$key}->{format};
> -		if ($fmt && $fmt =~ /^pve-qm-(?:ide|scsi|virtio|sata)$/) {
> +		if ($fmt && $fmt =~ /^pve-qm-(?:ide|scsi|virtio|sata|nvme)$/) {
>  		    my $v = parse_drive($key, $value);
>  		    if (my $volid = filename_to_volume_id($vmid, $v->{file}, $v->{media})) {
>  			$v->{file} = $volid;
> @@ -4289,6 +4292,17 @@ sub vm_deviceplug {
>  	    warn $@ if $@;
>  	    die $err;
>          }
> +    } elsif ($deviceid =~ m/^(nvme)(\d+)$/) {
> +
> +	qemu_driveadd($storecfg, $vmid, $device);
> +
> +	my $devicefull = print_drivedevice_full($storecfg, $conf, $vmid, $device, $arch, $machine_type);

Would prefer having a word separator: my $device_full = ...

> +	eval { qemu_deviceadd($vmid, $devicefull); };
> +	if (my $err = $@) {
> +	    eval { qemu_drivedel($vmid, $deviceid); };
> +	    warn $@ if $@;
> +	    die $err;
> +        }
>      } elsif ($deviceid =~ m/^(net)(\d+)$/) {
>  	return if !qemu_netdevadd($vmid, $conf, $arch, $device, $deviceid);
>  
> @@ -4361,6 +4375,9 @@ sub vm_deviceunplug {
>  
>  	qemu_iothread_del($vmid, "virtioscsi$device->{index}", $device)
>  	    if $conf->{scsihw} && ($conf->{scsihw} eq 'virtio-scsi-single');
> +    } elsif ($deviceid =~ m/^(nvme)(\d+)$/) {
> +	qemu_devicedel($vmid, $deviceid);
> +	qemu_drivedel($vmid, $deviceid);
>      } elsif ($deviceid =~ m/^(net)(\d+)$/) {
>  	qemu_devicedel($vmid, $deviceid);
>  	qemu_devicedelverify($vmid, $deviceid);
> diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
> index 6e98c09..f05ad26 100644
> --- a/PVE/QemuServer/Drive.pm
> +++ b/PVE/QemuServer/Drive.pm
> @@ -33,6 +33,7 @@ PVE::JSONSchema::register_standard_option('pve-qm-image-format', {
>  
>  my $MAX_IDE_DISKS = 4;
>  my $MAX_SCSI_DISKS = 31;
> +my $MAX_NVME_DISKS = 8;
>  my $MAX_VIRTIO_DISKS = 16;
>  our $MAX_SATA_DISKS = 6;
>  our $MAX_UNUSED_DISKS = 256;
> @@ -315,6 +316,20 @@ my $scsidesc = {
>  };
>  PVE::JSONSchema::register_standard_option("pve-qm-scsi", $scsidesc);
>  
> +my $nvme_fmt = {
> +    %drivedesc_base,
> +    %ssd_fmt,
> +    %wwn_fmt,
> +};
> +
> +my $nvmedesc = {

The existing `foodesc` use is ugly, not sure how much I'd care about style
compat here though, so IMO it would be slightly nicer to use:
my $nvme_desc = ...

> +    optional => 1,
> +    type => 'string', format => $nvme_fmt,
> +    description => "Use volume as NVME disk (n is 0 to " . ($MAX_NVME_DISKS -1) . ").",
> +};
> +
> +PVE::JSONSchema::register_standard_option("pve-qm-nvme", $nvmedesc);
> +
>  my $sata_fmt = {
>      %drivedesc_base,
>      %ssd_fmt,
> @@ -515,6 +530,11 @@ for (my $i = 0; $i < $MAX_SCSI_DISKS; $i++)  {
>      $drivedesc_hash_with_alloc->{"scsi$i"} = $desc_with_alloc->('scsi', $scsidesc);
>  }
>  
> +for (my $i = 0; $i < $MAX_NVME_DISKS; $i++)  {
> +    $drivedesc_hash->{"nvme$i"} = $nvmedesc;
> +}
> +
> +
>  for (my $i = 0; $i < $MAX_VIRTIO_DISKS; $i++)  {
>      $drivedesc_hash->{"virtio$i"} = $virtiodesc;
>      $drivedesc_hash_with_alloc->{"virtio$i"} = $desc_with_alloc->('virtio', $virtiodesc);
> @@ -541,6 +561,7 @@ sub valid_drive_names {
>              (map { "scsi$_" } (0 .. ($MAX_SCSI_DISKS - 1))),
>              (map { "virtio$_" } (0 .. ($MAX_VIRTIO_DISKS - 1))),
>              (map { "sata$_" } (0 .. ($MAX_SATA_DISKS - 1))),
> +            (map { "nvme$_" } (0 .. ($MAX_NVME_DISKS - 1))),
>              'efidisk0',
>              'tpmstate0');
>  }



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


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-07-31 15:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-31 13:16 [pve-devel] [WIP qemu-server, manager 0/3] nvme-emulation, current state Aaron Lauterer
2024-07-31 13:40 ` [pve-devel] [WIP qemu-server 1/3] fix #2255: add support for nvme emulation Aaron Lauterer
2024-07-31 13:40   ` [pve-devel] [WIP qemu-server 2/3] nvme: make it somewhat work with current version Aaron Lauterer
2024-07-31 13:40   ` [pve-devel] [WIP manager 3/3] gui: add nvme as a bus type for creating disks Aaron Lauterer
2024-07-31 15:30   ` [pve-devel] [WIP qemu-server 1/3] fix #2255: add support for nvme emulation Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal