[Qemu-block] [PATCH 2/3] nvme: ensure the num_queues is not zero

[Li Qiang]

When it is zero, it causes segv.
Using following command:

"-drive file=//home/test/test1.img,if=none,id=id0
-device nvme,drive=id0,serial=test,num_queues=0"
causes following Backtrack:

Thread 4 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffe9735700 (LWP 30952)]
0x55a7a77c in nvme_start_ctrl (n=0x577473f0) at hw/block/nvme.c:825
825 if (unlikely(n->cq[0])) {
(gdb) bt
0  0x55a7a77c in nvme_start_ctrl (n=0x577473f0)
at hw/block/nvme.c:825
1  0x55a7af7f in nvme_write_bar (n=0x577473f0, offset=20,
data=4587521, size=4) at hw/block/nvme.c:969
2  0x55a7b81a in nvme_mmio_write (opaque=0x577473f0, addr=20,
data=4587521, size=4) at hw/block/nvme.c:1163
3  0x55869236 in memory_region_write_accessor (mr=0x57747cd0,
addr=20, value=0x7fffe97320f8, size=4, shift=0, mask=4294967295, attrs=...)
at /home/test/qemu1/qemu/memory.c:502
4  0x55869446 in access_with_adjusted_size (addr=20,
value=0x7fffe97320f8, size=4, access_size_min=2, access_size_max=8,
access_fn=0x5586914d ,
mr=0x57747cd0, attrs=...) at /home/test/qemu1/qemu/memory.c:568
5  0x5586c479 in memory_region_dispatch_write (mr=0x57747cd0,
addr=20, data=4587521, size=4, attrs=...)
at /home/test/qemu1/qemu/memory.c:1499
6  0x558030af in flatview_write_continue (fv=0x7fffe0061130,
addr=4273930260, attrs=..., buf=0x77ff0028 "\001", len=4, addr1=20,
l=4, mr=0x57747cd0) at /home/test/qemu1/qemu/exec.c:3234
7  0x558031f9 in flatview_write (fv=0x7fffe0061130, addr=4273930260,
attrs=..., buf=0x77ff0028 "\001", len=4)
at /home/test/qemu1/qemu/exec.c:3273
8  0x558034ff in address_space_write (
---Type  to continue, or q  to quit---
as=0x56758480 , addr=4273930260, attrs=...,
buf=0x77ff0028 "\001", len=4) at /home/test/qemu1/qemu/exec.c:3363
9  0x55803550 in address_space_rw (
as=0x56758480 , addr=4273930260, attrs=...,
buf=0x77ff0028 "\001", len=4, is_write=true)
at /home/test/qemu1/qemu/exec.c:3374
10 0x558884a1 in kvm_cpu_exec (cpu=0x56920e40)
at /home/test/qemu1/qemu/accel/kvm/kvm-all.c:2031
11 0x5584cd9d in qemu_kvm_cpu_thread_fn (arg=0x56920e40)
at /home/test/qemu1/qemu/cpus.c:1281
12 0x55dbaf6d in qemu_thread_start (args=0x569438a0)
at util/qemu-thread-posix.c:502
13 0x75dc86db in start_thread (arg=0x7fffe9735700)
at pthread_create.c:463
14 0x75af188f in clone ()
at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Signed-off-by: Li Qiang 
Reviewed-by: Philippe Mathieu-Daud?? 
---
 hw/block/nvme.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index f206391e8e..0b77b49b36 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1208,6 +1208,11 @@ static void nvme_realize(PCIDevice *pci_dev, Error 
**errp)
 int64_t bs_size;
 uint8_t *pci_conf;
 
+if (!n->num_queues) {
+error_setg(errp, "num_queues can't be zero");
+return;
+}
+
 if (!n->conf.blk) {
 error_setg(errp, "drive property not set");
 return;
-- 
2.17.1

[Qemu-block] [PATCH 3/3] nvme: use pci_dev directly in nvme_realize

[Li Qiang]

There is no need to make another reference.

Signed-off-by: Li Qiang 
Reviewed-by: Max Reitz 
Reviewed-by: Philippe Mathieu-Daud?? 
---
 hw/block/nvme.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 0b77b49b36..8325b5e88a 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1238,7 +1238,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
 pci_conf[PCI_INTERRUPT_PIN] = 1;
 pci_config_set_prog_interface(pci_dev->config, 0x2);
 pci_config_set_class(pci_dev->config, PCI_CLASS_STORAGE_EXPRESS);
-pcie_endpoint_cap_init(>parent_obj, 0x80);
+pcie_endpoint_cap_init(pci_dev, 0x80);
 
 n->num_namespaces = 1;
 n->reg_size = pow2ceil(0x1004 + 2 * (n->num_queues + 1) * 4);
@@ -1250,10 +1250,10 @@ static void nvme_realize(PCIDevice *pci_dev, Error 
**errp)
 
 memory_region_init_io(>iomem, OBJECT(n), _mmio_ops, n,
   "nvme", n->reg_size);
-pci_register_bar(>parent_obj, 0,
+pci_register_bar(pci_dev, 0,
 PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64,
 >iomem);
-msix_init_exclusive_bar(>parent_obj, n->num_queues, 4, NULL);
+msix_init_exclusive_bar(pci_dev, n->num_queues, 4, NULL);
 
 id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
 id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
@@ -1308,7 +1308,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
 n->cmbuf = g_malloc0(NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
 memory_region_init_io(>ctrl_mem, OBJECT(n), _cmb_ops, n,
   "nvme-cmb", NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
-pci_register_bar(>parent_obj, NVME_CMBLOC_BIR(n->bar.cmbloc),
+pci_register_bar(pci_dev, NVME_CMBLOC_BIR(n->bar.cmbloc),
 PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64 |
 PCI_BASE_ADDRESS_MEM_PREFETCH, >ctrl_mem);
 
-- 
2.17.1

[Qemu-block] [PATCH 1/3] nvme: use TYPE_NVME instead of constant string

[Li Qiang]

Signed-off-by: Li Qiang 
Reviewed-by: Max Reitz 
Reviewed-by: Philippe Mathieu-Daud?? 
---
 hw/block/nvme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 7c8c63e8f5..f206391e8e 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1381,7 +1381,7 @@ static void nvme_instance_init(Object *obj)
 }
 
 static const TypeInfo nvme_info = {
-.name  = "nvme",
+.name  = TYPE_NVME,
 .parent= TYPE_PCI_DEVICE,
 .instance_size = sizeof(NvmeCtrl),
 .class_init= nvme_class_init,
-- 
2.17.1

[Qemu-block] [PATCH 0/3] nvme small fix

[Li Qiang]

This patchset contains small fix.

Change since v2:
For patch 2:
1. add nvme command
2. check num_queues first

Change since v1: 

1. drop the patch of checking return value of msix_init_exclusive_bar
2. return when nvme's num_queues configuration is 0

Li Qiang (3):
  nvme: use TYPE_NVME instead of constant string
  nvme: ensure the num_queues is not zero
  nvme: use pci_dev directly in nvme_realize

 hw/block/nvme.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

-- 
2.17.1

Re: [Qemu-block] [PATCH v4 00/21] nbd: add qemu-nbd --list

[Richard W.M. Jones]

Attached is a NON-working patch to nbdkit-partitioning-plugin which
adds logical partition support.  I don't think I've fully understood
how the EBR fields are supposed to be initialized (or else they don't
work how is described in online documentation).  This actually causes
parted to print an internal error, while fdisk gives a warning and the
size of the logical partition is wrong.

Anyway I've run out of time to work on it this weekend, but I may be
able to have another look next week.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
>From 8d6dd4288064e9b880eccb1a6b1b7a6b03f2ba96 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" 
Date: Sat, 19 Jan 2019 10:30:28 +
Subject: [PATCH] partitioning: Support MBR logical partitions.

XXX NEEDS TESTS
---
 .../nbdkit-partitioning-plugin.pod|  25 +---
 plugins/partitioning/virtual-disk.h   |  15 +-
 plugins/partitioning/partition-gpt.c  |   2 +-
 plugins/partitioning/partition-mbr.c  | 134 +++---
 plugins/partitioning/partitioning.c   |  31 ++--
 plugins/partitioning/virtual-disk.c   |  42 +-
 6 files changed, 184 insertions(+), 65 deletions(-)

diff --git a/plugins/partitioning/nbdkit-partitioning-plugin.pod 
b/plugins/partitioning/nbdkit-partitioning-plugin.pod
index 57a1133..edb0776 100644
--- a/plugins/partitioning/nbdkit-partitioning-plugin.pod
+++ b/plugins/partitioning/nbdkit-partitioning-plugin.pod
@@ -27,23 +27,12 @@ access use the I<-r> flag.
 
 =head2 Partition table type
 
-You can choose either an MBR partition table, which is limited to 4
-partitions, or a GPT partition table.  In theory GPT supports an
-unlimited number of partitions.
-
-The rule for selecting the partition table type is:
+Using the C parameter you can choose either an MBR or
+a GPT partition table.  If this parameter is I present then:
 
 =over 4
 
-=item C parameter on the command line
-
-⇒ MBR is selected
-
-=item C parameter on the command line
-
-⇒ GPT is selected
-
-=item else, number of files E 4
+=item number of files E 4
 
 ⇒ GPT
 
@@ -131,7 +120,9 @@ C<./> (absolute paths do not need modification).
 =item B
 
 Add an MBR (DOS-style) partition table.  The MBR format is maximally
-compatible with all clients, but only supports up to 4 partitions.
+compatible with all clients.  If there are E 4 partitions then an
+extended partition is created
+(L).
 
 =item B
 
@@ -163,10 +154,6 @@ a Linux filesystem.
 
 =head1 LIMITS
 
-This plugin only supports B MBR partitions, hence the limit
-of 4 partitions with MBR.  This might be increased in future if we
-implement support for logical/extended MBR partitions.
-
 Although this plugin can create GPT partition tables containing more
 than 128 GPT partitions (in fact, unlimited numbers of partitions),
 some clients will not be able to handle this.
diff --git a/plugins/partitioning/virtual-disk.h 
b/plugins/partitioning/virtual-disk.h
index 3860f46..f59df70 100644
--- a/plugins/partitioning/virtual-disk.h
+++ b/plugins/partitioning/virtual-disk.h
@@ -34,6 +34,7 @@
 #ifndef NBDKIT_VIRTUAL_DISK_H
 #define NBDKIT_VIRTUAL_DISK_H
 
+#include 
 #include 
 #include 
 
@@ -103,7 +104,7 @@ extern struct file *files;
 extern size_t nr_files;
 
 extern struct regions regions;
-extern unsigned char *primary, *secondary;
+extern unsigned char *primary, *secondary, **ebr;
 
 /* Main entry point called after files array has been populated. */
 extern int create_virtual_disk_layout (void);
@@ -114,16 +115,16 @@ extern int create_virtual_disk_layout (void);
 extern int parse_guid (const char *str, char *out)
   __attribute__((__nonnull__ (1, 2)));
 
-/* Internal functions for creating MBR and GPT layouts.  These are
- * published here because the GPT code calls into the MBR code, but
- * are not meant to be called from the main plugin.
+/* Internal function for creating a single MBR PTE.  The GPT code
+ * calls this for creating the protective MBR.
  */
-extern void create_mbr_partition_table (unsigned char *out)
-  __attribute__((__nonnull__ (1)));
 extern void create_mbr_partition_table_entry (const struct region *,
-  int bootable, int partition_id,
+  bool bootable, int partition_id,
   unsigned char *)
   __attribute__((__nonnull__ (1, 4)));
+
+/* Create MBR or GPT layout. */
+extern void create_mbr_layout (void);
 extern void create_gpt_layout (void);
 
 #endif /* NBDKIT_VIRTUAL_DISK_H */
diff --git a/plugins/partitioning/partition-gpt.c 
b/plugins/partitioning/partition-gpt.c
index 5fb7602..be52e72 100644
--- a/plugins/partitioning/partition-gpt.c
+++ b/plugins/partitioning/partition-gpt.c
@@ 

Re: [Qemu-block] [PATCH v4 00/21] nbd: add qemu-nbd --list

[Richard W.M. Jones]

On Fri, Jan 18, 2019 at 04:47:42PM -0600, Eric Blake wrote:
> It matches the code, but I just learned the code is buggy for anything
> larger than 5.  According to
> http://tldp.org/HOWTO/Large-Disk-HOWTO-13.html, MBR Extended/Logical
> partitions form a linked-list, something like:
> 
> MBR:  EBR1  EBR2
> +--+  +--+  +--+
> + Part 1   +   |->+ Part 5   +   |->+ Part 6   +
> + Part 2   +   |  + Next |  + 0+
> + Part 3   +   |  + 0+  + 0+
> + Part 4 ---  + 0+  + 0+
> +--+  +--+  +--+

Wikipedia's description is a bit better:

https://en.wikipedia.org/wiki/Extended_boot_record

In fact it's not strictly a linked list because the start record of
each entry is relative to the extended partition start.  It's all a
bit weird.

But you're right, nbdkit can neither read (nbdkit-partition-filter)
nor write (nbdkit-partitioning-plugin) MBR logical partitions so we
can't test interop or tell people to use nbdkit for this corner case.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW